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

fix(side-navbar): keyboard a11y #222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gaurav5430
Copy link

replace div with button to allow activation using space and enter.

This fixes one of the issues mentioned in #221

replace div with button to allow activation using space and enter

Signed-off-by: Gaurav Gupta<[email protected]>
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Jul 20, 2022
@github-actions github-actions bot closed this Jul 28, 2022
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Aug 16, 2023
@agilgur5 agilgur5 reopened this Aug 16, 2023
@agilgur5
Copy link
Contributor

Looks like DCO is missing a space (between name and the <)

@gaurav5430
Copy link
Author

Looks like DCO is missing a space (between name and the <)

Thanks for reopening this, I might find time to fix the DCO issue and rebase this

@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Oct 16, 2023
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Oct 16, 2023

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Dec 16, 2023
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Dec 16, 2023

This comment was marked as resolved.

@github-actions github-actions bot added the problem/stale This has not had a response in some time label Feb 15, 2024
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Feb 15, 2024
@michaelfresco
Copy link

I think that this would also make the buttons clickable with vimium!

@@ -35,10 +35,10 @@ export const NavBar: React.FunctionComponent<NavBarProps> = (props: NavBarProps,
<div className='nav-bar__version'>{props.version && props.version()}</div>
{(props.items || []).map((item) => (
<Tooltip content={item.title} placement='right' arrow={true} key={item.path + item.title}>
<div className={classNames('nav-bar__item', { active: isActiveRoute(locationPath, item.path) })}
<button type="button" className={classNames('nav-bar__item', { active: isActiveRoute(locationPath, item.path) })}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can actually just use a <Link> element from react-router, which would also solve #166

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