-
Notifications
You must be signed in to change notification settings - Fork 462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sure copy button is tab-accessible and visible on tab #3192
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-ran the build stage that failed - it didn't immediately look relevant to this PR. One small nit that may well be personal preference, but looks good.
@@ -20,6 +20,6 @@ | |||
cursor: pointer; | |||
} | |||
|
|||
::deep:hover .defaultHidden { | |||
::deep:hover .defaultHidden, ::deep:focus-within .defaultHidden { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I prefer to keep selectors on separate lines for easier scanning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally it confuses me more but I wouldn’t mind changing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😂 Personal preferences in coding styles are fun 😂
If I see one line I generally assume its just one (long) selector and then can vertically scan the selectors to see the differences with any that are grouped with it (e.g. the ::deep:hover
would stand out from the ::deep:focus-within
since they'd be aligned and right on top of each other).
I don't think we have any standards around this for the repo, so I'm fine either way. Just throwing it out there.
Co-authored-by: Adam Ratzman <[email protected]>
Co-authored-by: Adam Ratzman <[email protected]> (cherry picked from commit 2bba2ad)
* Make sure copy button is tab-accessible and visible on tab (#3192) Co-authored-by: Adam Ratzman <[email protected]> (cherry picked from commit 2bba2ad) * Avoid reconnection modal taking up entire screen, not being able to i… (#3189) * Avoid reconnection modal taking up entire screen, not being able to interact with page * add slight padding, shadow --------- Co-authored-by: Adam Ratzman <[email protected]> (cherry picked from commit 87fab1e) * Force log level select position to be below (#3406) Co-authored-by: Adam Ratzman <[email protected]> (cherry picked from commit 5a6856d) * Re-focus view button when closing details panel (#3368) * Re-focus button when closing details panel * Add internal id for log entries, make sure to null element id on detail view closed * Make sure view button is selected only after a user action --------- Co-authored-by: Adam Ratzman <[email protected]> (cherry picked from commit e784c91)
We can add a tabindex to GridValue buttons to make sure they are keyboard navigable in data grids, and we can use the focus-within event to set opacity on keyboard focus
Fixes #3175
Microsoft Reviewers: Open in CodeFlow