Skip to content

Use ID of operating sysems instead of name in selectors#13506

Merged
parlough merged 3 commits into
mainfrom
fix/os-selector-ids
Jun 16, 2026
Merged

Use ID of operating sysems instead of name in selectors#13506
parlough merged 3 commits into
mainfrom
fix/os-selector-ids

Conversation

@parlough

@parlough parlough commented Jun 16, 2026

Copy link
Copy Markdown
Member

Fixes the OS selector on pages such as https://docs.flutter.dev/install/manual page after I didn't account for the changes to OperatingSystem.name in 156b8c3.

Staged: https://flutter-docs-prod--docs-pr13506-fix-os-selector-ids-5rqp5zfu.web.app/install/manual

@parlough parlough requested a review from domesticmouse June 16, 2026 09:35
@parlough parlough requested review from a team and sfshaza2 as code owners June 16, 2026 09:35

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the OsSelector component to use os.id instead of os.name for CSS classes, element IDs, and data attributes. The review feedback highlights a variable shadowing issue where the loop variable os shadows the method parameter os, which could cause confusion, and suggests renaming the loop variable to improve code readability.

Comment thread sites/docs/lib/src/components/common/client/os_selector.dart Outdated
@flutter-website-bot

flutter-website-bot commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Staged preview of the updated docs.flutter.dev site (updated for commit a6a1f58):

https://flutter-docs-prod--docs-pr13506-fix-os-selector-ids-5rqp5zfu.web.app

@flutter-website-bot

flutter-website-bot commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Staged preview of the updated flutter.dev site (updated for commit a6a1f58):

https://flutter-dev-230821--www-pr13506-fix-os-selector-ids-v5uw3es9.web.app

@sfshaza2 sfshaza2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@parlough parlough merged commit bfce650 into main Jun 16, 2026
15 checks passed
@parlough parlough deleted the fix/os-selector-ids branch June 16, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants