-
Notifications
You must be signed in to change notification settings - Fork 594
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
Editor focus borders on keyboard navigation #10462
Editor focus borders on keyboard navigation #10462
Conversation
…n the default and high contrast themes. Includes: the menu bar, editor selection, simulator transport, sidebar toggles, download and editor bottom bar buttons.
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 like the general change here. I think it's worth exploring if we can use the focus-border theme var in more places (which will help if we ever develop more themes for this target). Is that somehting you've already tried?
If we need to change that variable's color to something more universally applicable, we can definitely do that!
Will follow up on the "UI Questions" in the description separately. I'll need a bit of time to think those over.
@@ -98,6 +98,11 @@ | |||
} | |||
} | |||
|
|||
.common-button:focus-visible { | |||
outline: 2px solid var(--pxt-tertiary-foreground); |
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.
Did you experiment with var(--pxt-focus-border) for this color? Ideally, we'd use that in most cases where we're showing focus like this. If it means we have to change the color --pxt-focus-border is set to, that's definitely an option.
To answer some of the questions you asked in the description: For "Focus Styles", my thoughts are that inverting the colors will be too jarring, and an animation might be too specific to this scenario, but perhaps using a different color for the focus border here could work. Funnily enough, while I agree with you that the blue border was less visible on the new-project card, I think it's more visible on this download button. My preference would be to keep it as-is with the blue border. Or, if we do change that color to be purple per our discussions in #10446, then adding some special-case css in the theme overrides file to set the focus border on "primary buttons" (aka the purple ones) to be blue. Putting it in the overrides file ensures that it won't affect other themes or targets. For "Hover styles" I actually think keeping the hover styling in addition to the focus border makes it clearer what's selected, so my (not very strong) preference would be to leave them combined. For "Menubar Patterns" I defer to your recommendation. With all the testing you & your team have done, I think you have more context on what's best here. |
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.
LGTM but I'll hold off on merging until I hear back from you regarding #10462 (comment), in case any of those answers affect your preferences for this change. (I think it's fine as-is, for what it's worth.)
I'm going to merge this, but feel free to follow-up with additional changes if #10462 (comment) affects anything you want to do here. |
This makes the focus borders on buttons thicker and more noticeable in the default and high contrast themes.
Includes: the menu bar, editor selection, simulator transport, sidebar toggles, download and editor bottom bar buttons.
Does not include improvements to the simulator or workspace (Blockly et al).
Addresses microsoft/pxt-microbit#5538 (comment)
There is discussion below around microsoft/pxt-microbit#6082
Related work:
Demo: https://editor-tab-focus-borders.review-pxt.pages.dev/#editor
Old appearance
New appearance
New appearance in high contrast mode
UI questions
Focus styles
The download button focus highlight, is less noticeable than the other highlights, due to the strong contrast already present on that button. We should discuss how to make this more obvious, particularly as this is the first tab stop after the workspace, and could be quite distant spatially from where the user is currently looking.
We've found in user testing with the work-in-progress Blockly keyboard navigation, folks can often not notice that they've tabbed away from the workspace as they're fixated there.
Current focus appearance

One option would be to invert the colours of the button on focus, to make the visual change more prominent. This could be a general principle with the primary theme buttons, or just applied for this specific case where there is an issue. For example:

Another option, if we treat it as a specific case and do not extend it to ordinary button focus, would be to give it a light transition animation as the user focuses the download button, to guide the user's gaze. We'll collect further input and suggestions here.
Hover styles
The Semantic UI components represent focus style and hover style equivalently, which is inadequate on its own for accessibility purposes, but combined with the selection outline may still be desirable- there is no clear best practice here, just whatever is clear and consistent. For instance, with hover styles:
Screen.Recording.2025-03-24.at.16.28.52.mov
Without:
Screen.Recording.2025-03-24.at.16.32.07.mov
From a practical implementation perspective, we need to make a decision on this because there's a transition delay for the background that means it appears slightly after the outline. If we choose to keep hover animations on focus, making hover animations consistent will require extending the transitions in
react-common/controls/Button
. If we separate focus and hover visual styles, we have a simpler implementation.The version in this PR uses the hover styling, but we've not done anything about the transition delay.
Menubar Patterns
To simplify navigating with tab we suggest considering making the simulator a single tab stop using the menubar pattern. What do you think?
See: https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-navigation/
We can look at this on a separate PR.
Wider changes
We've not considered pulling up/centralising these style changes and would welcome input on this.
CC @microbit-matt-hillsdon @microbit-robert