Skip to content
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

Handle group focus highlights in scenery #708

Closed
jessegreenberg opened this issue Nov 7, 2017 · 16 comments
Closed

Handle group focus highlights in scenery #708

jessegreenberg opened this issue Nov 7, 2017 · 16 comments

Comments

@jessegreenberg
Copy link
Contributor

In phetsims/scenery-phet#341, we added a custom highlight around NumberControls when they have focus, so that they look something like
capture

After a11y meeting on 11/7/17, it sounds like this kind of highlight will should be used for any grouped elements such as radio buttons, carousels, combo boxes, and menus. It would be great if scenery could generally support this.

@zepumph @mbarlow12 lets discuss this next a11y meeting.

@jessegreenberg
Copy link
Contributor Author

Maybe nodes can have a group focus highlight that is made visible whenever a descendant node receives focus, handled in FocusOverlay?

@zepumph zepumph removed their assignment Nov 9, 2017
@jessegreenberg
Copy link
Contributor Author

Discussed in 11/9/17 accessibility meeting. Lets discuss again at a design meeting for keyboard navigation. There are concerns that adding another focus highlight will require additional spacing for all Nodes that use this. The more complex the focus highlight is, the more space it will take up. We may not have that space for a lot of common code components.

There was also concern that a "grouping" highlight wouldn't solve the problem that these kinds of UI inputs don't indicate different keyboard interaction. Perhaps that would be better solved with a different UI solution that isn't specific to a11y and keyboard navigation.

@jessegreenberg
Copy link
Contributor Author

Discussed in 11/14/17 accessibility meeting: We are going to hold off on this and not indicate element grouping with focus highlights since it paints us into a corner and we would have to make sure there is additional spacing for this in every set of grouped elements. We will handle this on a case by case basis to ensure that the interaction pattern for a group of elements is clear. Closing.

@zepumph
Copy link
Member

zepumph commented Nov 14, 2017

We will handle this on a case by case basis to ensure that the interaction pattern for a group of elements is clear.

In addition, I think we decided that this "case by case" basis may be through focusHighlight grouping, or through universal design in the sim itself, depending on what the sim needs.

@jessegreenberg
Copy link
Contributor Author

In a 12/5 a11y design meeting we decided to add this outer highlight for all radio buttons and scenery-phet/NumberControl, but there might be others in the future. I think this should be handled generally in scenery, @zepumph @mbarlow12 what do you think?

@zepumph
Copy link
Member

zepumph commented Dec 6, 2017

Sounds awesome. We could have a node have a flag to mark a parent node as a group highlight.
Picture this structure:

screen
   panel
      button
      button

the Panel Node can have an option groupHighlight :true. which will automatically surround the panel in a group focusHighlight.

Then in FocusOverlay, we just have to add a step that iterates over parent accessibleInstances and searches for groupHighlight opt ins.

Perhaps we will want flexibility to customize the groupHighlight, similar to FocusHighlightPath, so potentially the option will be of type {{boolean}}, or {{node}}, or {{shape}}, or allow any of these {{boolean|node}}.

@jessegreenberg
Copy link
Contributor Author

That seems great @zepumph, thats what I was thinking as well.

@jessegreenberg
Copy link
Contributor Author

Oops, meant to reopen this.

@jessegreenberg
Copy link
Contributor Author

Initial pass done in the above commits. Right now, there is only support for a flag that enables a group focus highlight. Remaining work includes ability to pass a custom Shape or Node group focus highlight, and potentially support for multiple group focus highlights in case multiple ancestors need to show a group highlight if a node has focus.

@mbarlow12
Copy link
Contributor

@jessegreenberg @zepumph Is this feature still in development at all? It seems like the GroupFocusHighlight has been working well for some time—can we close?

@jessegreenberg
Copy link
Contributor Author

Thanks @mbarlow12, yes this can be closed. I updated some documentation around this issue. The enhancements listed in #708 (comment) have not been necessary since this issue was opened and I don't want to reconsider the API until they are required for some reason.

@zepumph
Copy link
Member

zepumph commented Sep 22, 2018

There is one more todo I see in Accessibility.js that applies to this issue I think. What do you think?

@jessegreenberg
Copy link
Contributor Author

The TODO is still there and is

TODO: Support more than one group focus highlight (multiple ancestors could have groupFocusHighlight), see https://github.com/phetsims/scenery/issues/708

We haven't needed this for 4 years, but it would be nice to support for completion. Doubt it is something we will get to soon.

@jessegreenberg
Copy link
Contributor Author

The next steps will be done in #1608. Closing this one since we have supported groupFocusHighlight for a long time.

@phet-dev
Copy link
Contributor

Reopening because there is a TODO marked for this issue.

@zepumph
Copy link
Member

zepumph commented Feb 12, 2024

Just moved a TODO over to 1608.

@zepumph zepumph closed this as completed Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants