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

Feature: only add ARIA-LABEL if the element does not already has one #5298

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liorchamla
Copy link

@liorchamla liorchamla commented Jan 6, 2025

Hey folks, this is a wonderful project ! So first thing first, thanks to all contributors, the library is great. We are working a lot on a11y since a week, and the problem we have with Marker is that it always has the same aria-label derived from a default string Marker.Title. But in our use case, we want Markers to be self explainatory to our audience.

Our custom HTML element for the marker has a good aria-label but it is replaced automaticly when the Marker is added to the map (addTo() method).

So my proposition here is to say : If the element already has an aria-label attribute that makes sense in the userland context, let's not replace it with the default Marker.Title.

Edit

By the way, I just found that there was an issue related (sort of) to my PR : #355

Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.84%. Comparing base (0061f85) to head (9c8c62b).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5298   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files         282      282           
  Lines       38908    38910    +2     
  Branches     6829     6829           
=======================================
+ Hits        35735    35737    +2     
  Misses       3046     3046           
  Partials      127      127           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@liorchamla
Copy link
Author

By the way, I can't understand why spellcheck is failing in the CI while it works fine inside my CodeSpace :'(

If you have any advice on what is missing ?

src/ui/marker.ts Outdated Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Jan 7, 2025

This is related to awesome-maplibre project spelling issue there, I'll see if I can fix this quickly.

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.

2 participants