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

(feat) O3-3683: Add aria-labels to Start and End visit buttons in the visit header #1946

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

Vijaykv5
Copy link
Contributor

@Vijaykv5 Vijaykv5 commented Jul 30, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR adds aria-label property for the easy accessibility of Start a Visit and End visit button in the Navbar.

Related Issue

O3-3683

Other

cc : @jayasanka-sack

@Vijaykv5 Vijaykv5 changed the title (enhc) O3-3683 : Add aria-label property for buttons in Navbar (enhc) O3-3683 : Add aria-label property for buttons in navbar Jul 30, 2024
@denniskigen denniskigen changed the title (enhc) O3-3683 : Add aria-label property for buttons in navbar (feat) O3-3683: Add aria-labels to Start and End visit buttons in the visit header Jul 31, 2024
@jayasanka-sack
Copy link
Member

@denniskigen @ibacher we need to provide CSS selectors for the onboarding tutorials. It's straightforward for most elements since we can use extension IDs and other selectors. However Vijay is having a hard time querying some elements like the start visit button. What’s the best way to define selectors for them? Using aria-label isn’t ideal because it changes with the locale. Maybe we should add an ID?

@ibacher
Copy link
Member

ibacher commented Aug 1, 2024

Bah! It's as easy as [data-extension-slot-name="visit-header-right-slot"] + button</s>.

In all seriousness, it makes sense to include something that makes it easier to select the appropriate value (though the above will sort of work, though it may also select the "End Visit" button). However, I'm hesitant about defining ids. While these are easy to work with from an element selection standpoint, they are extremely hard to work with from a microfrontend stand-point (because we need to ensure that no other MF renders an element with the same id, since ids must be unique within a DOM tree).

Maybe we should add a data-openmrs-role attribute to button those buttons so the selector could be button[data-openmrs-role="Start Visit"].

@jayasanka-sack
Copy link
Member

Thanks @ibacher ! @Vijaykv5 let's move forward with data-openmrs-role attribute when we specifically need to select an element.

Even though it is unrelated to selectors, I'll merge this PR as it helps to improve accessibility.

@jayasanka-sack jayasanka-sack merged commit 866afa7 into openmrs:main Aug 1, 2024
5 of 6 checks passed
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.

4 participants