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

Refactor NavBar component to better support accessibility changes #3250

Open
1 task
tespin opened this issue Oct 16, 2024 · 4 comments
Open
1 task

Refactor NavBar component to better support accessibility changes #3250

tespin opened this issue Oct 16, 2024 · 4 comments
Labels
Area:Accessibility Category for accessibility related features and bugs Enhancement Priority:High

Comments

@tespin
Copy link
Contributor

tespin commented Oct 16, 2024

Increasing Access

Work is being done in #3220 to add keyboard navigation to the p5.js NavBar. The current design of the component can support some of the changes being made, but a few interactions require a bit more complexity between the individual parts. A refactor would support a more ARIA-compliant component and make it more legible and flexible to work with as a developer.

Feature enhancement details

Below are changes that can be considered in a refactor. Some respond to specific functions of the menubar while others are design recommendations. @lindapaiste would love your input on some of these changes as an author of the component!

Components that contain menu items should be aware of each item in that menu.
Currently the NavBar is composed by piecing together the NavDropdownMenu and NavMenuItem components. However, neither the NavBar nor NavDropdownMenu components internally keep track of their respective menu items. This information is needed to properly traverse each menu item with keyboard controls.

I’m following this implementation where menu items are stored in a parent menu context as a Set of MenuItem nodes. Each child menu item then adds itself to the Set via useEffect. Open to other implementations!

Extract keyboard behaviors into a reusable hook.
I experimented with the useKeyDownHandlers hook but had some glitchy interactions — jumping over multiple menu items, conflicting with component-specific events, etc. I could be using it wrong, but I'm wondering if it makes sense to author another hook for menubar-specific interactions. Maybe something like:

export function useKeyboardNavigation(menuitems) {
  const [currentIndex, setCurrentIndex] = useState(0);

  const first = () => setCurrentIndex(0);
  // last, next, previous, etc.
  ...

  const handleKeyDown = (e) => {
	e.stopPropagation();
	switch (e.key) {
  	case "Home":
    	e.preventDefault();
    	first();
    	break;
	// as well as going to the end of the list, next item, previous, matching chars, etc.
        ...
  	default:
    	break;
	}
  };

  return [currentIndex, setCurrentIndex, handleKeyDown];
}

We can then build on top of the hook to take care of specific behaviors in certain components.

Refactor the Mobile version of the navigation.
I’ll have to look into this more, it looks like the mobile version is just a single combined list of all the menu items. Maybe it won’t need more than an encapsulating menu component with keyboard controls, but I’m not sure how the layout shift affects screen readers.

Change component names from NavBar to Menubar.
Currently the related components are named NavBar, NavDropdownMenu, etc. Semantically, the NavBar component more closely resembles a menubar than it does navigation. MDN says: "An example of a web-based menubar is the horizontal menu bar that reads "File Edit View Insert Format", etc., which is usually visible under the document name in a Google doc.” Since a similar pattern is used in the p5.js editor, I suggest renaming the components from NavBar to Menubar. Related components can be renamed from NavDropdownMenu to Menu, NavMenuItem to MenuItem, etc. Handler methods and contexts should also be appropriately renamed.

Further exploration:

Add a MenuTrigger (and maybe Submenu / SubmenuTrigger components).
Speaking of related components, it may be easier to further break down existing components since some ARIA interactions for a menubar are only executed by specific elements. Specifically, we can add a MenuTrigger component that keeps its key events separate from the events of other components, such as Menu. We can also consider adding a Submenu component in case there’s a feature that might use a submenu in the future.

I've already started some of this work as part of the PR I linked above, so I'm happy to continue taking this on! Hoping to get others feedback in the meantime.

@kemkartanya
Copy link

@processing-bot claim

@raclim raclim added Priority:High Area:Accessibility Category for accessibility related features and bugs labels Nov 6, 2024
@ujjwaldubey1
Copy link

ujjwaldubey1 commented Nov 13, 2024

@raclim I find this great to work on so , I have strted to work on this .

@tespin
Copy link
Contributor Author

tespin commented Nov 13, 2024

@raclim I find this great to work on so , I have strted to work on this .

hi @ujjwaldubey1 , i’ve already started on this issue as part of a PR i linked — just wanted to let you know before you started working on it.

i’m also hoping for more discussion before making some of the larger design changes mentioned in this issue. feel free to add any thoughts!

@ujjwaldubey1
Copy link

Okay!! Let's build togather .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Accessibility Category for accessibility related features and bugs Enhancement Priority:High
Projects
None yet
Development

No branches or pull requests

4 participants