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: Accessibility issue #1509

Merged

Conversation

GuillaumeRozier
Copy link
Contributor

Propose to fix issue #1353

Checklist
Description of change

Removed role="button"from https://github.com/Jasig/uPortal/blob/master/uPortal-webapp/src/main/resources/layout/theme/respondr/navigation.xsl#L375
Tested with VoiceOver.

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Thanks @GuillaumeRozier! 🙇‍♂️
Welcome to the uPortal community! 👋
These changes look good to me. 👍

@ChristianMurphy ChristianMurphy added this to the 5.4.2 milestone Jan 30, 2019
@ChristianMurphy ChristianMurphy added enhancement Improvement good first issue Good for newcomers labels Jan 30, 2019
Copy link
Contributor

@cousquer cousquer left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 fully respect two rules of WAI-ARIA:

  • first rule is: if you don't need ARIA, Don't use it
  • second rule is: "if it goes somewhere, use a link <a href="#somewhere>, if it does something, use a button <button >

Enjoying asynchronously a beer 🍻 with you @GuillaumeRozier
Many thanks! 😉

@drewwills
Copy link
Contributor

Thank you @GuillaumeRozier!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants