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

Fixed improper navigation semantics in accessibility contexts #198

Merged
merged 2 commits into from
Feb 9, 2023
Merged

Fixed improper navigation semantics in accessibility contexts #198

merged 2 commits into from
Feb 9, 2023

Conversation

ivyrze
Copy link
Contributor

@ivyrze ivyrze commented Feb 8, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

Adds an aria-current="page" to active menu links and breadcrumb segments, resolving #197.

The menu tests as previously designed have more than one page marked as active, which means aria-current is being used improperly in those test scenarios. I'm assuming that in real-world use, this would not be the case?

@Ocramius Ocramius changed the base branch from 2.26.x to 2.27.x February 8, 2023 18:57
@Ocramius Ocramius added this to the 2.27.0 milestone Feb 8, 2023
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Besides the small Psalm misalignment, this LGTM!

Thanks @ivyrze. Give the baseline a try, and if it fails, I'll pick it from here, just let me know.

src/Helper/Navigation/Breadcrumbs.php Show resolved Hide resolved
src/Helper/Navigation/Breadcrumbs.php Show resolved Hide resolved
@ivyrze
Copy link
Contributor Author

ivyrze commented Feb 9, 2023

Thanks for the pointer @Ocramius!

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Thanks @ivyrze!

@Ocramius Ocramius self-assigned this Feb 9, 2023
@Ocramius Ocramius linked an issue Feb 9, 2023 that may be closed by this pull request
@Ocramius Ocramius merged commit b7e66e1 into laminas:2.27.x Feb 9, 2023
@Ocramius Ocramius changed the title Fixed improper navigation semantics Fixed improper navigation semantics in accessibility contexts Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improper navigation semantics
2 participants