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.

Sign up for GitHub

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

Jump to bottom

Use a button instead of a link to remove a marker #5111

Merged
merged 4 commits into from
May 5, 2023
Merged

Use a button instead of a link to remove a marker #5111

merged 4 commits into from
May 5, 2023

Conversation

javierm
Copy link
Member

@javierm javierm commented May 2, 2023

References

Objectives

  • Improve the experience of people using screen readers when removing the map marker
  • Simplify the HTML rendering the button to remove a marker
  • Slightly simplify the JavaScript dealing with the button to remove a marker

@javierm javierm self-assigned this May 2, 2023
@javierm javierm force-pushed the remove_marker_accessibility branch 4 times, most recently from 7afd6bd to 3ad177f Compare May 2, 2023 14:44
@javierm javierm force-pushed the remove_marker_accessibility branch from 3ad177f to f0b014f Compare May 2, 2023 15:10
@taitus taitus self-assigned this May 4, 2023
@javierm javierm force-pushed the remove_marker_accessibility branch from f0b014f to 5723e6a Compare May 4, 2023 13:21
@javierm
Remove unused HTML class in link to remove marker
f4ef641 
It isn't used since commit c34aa54.
@javierm
Make map location HTML class names consistent
a145fdb 
We were using `map_location` in one place and
`location-map-remove-marker` in another one. We usually use dashes in
HTML class names, we don't say "location map" anywhere else.
@javierm
Set remove marker link margin-bottom in CSS
59ed975 
We were setting the margin-top property in CSS but the margin-bottom
property by adding a div with a `.margin-bottom` HTML class.
@javierm
Use a button instead of a link to remove a marker
8b14522 
Using a button for interactive elements is better, as explained in
commit 5311daa.

Since buttons with "type=button" do nothing by default, we no longer
need to call `preventDefault()` when clicking it.
@javierm javierm force-pushed the remove_marker_accessibility branch from 5723e6a to 8b14522 Compare May 4, 2023 13:32
Base automatically changed from map_refactoring to master May 5, 2023 13:21
@javierm javierm merged commit 287f483 into master May 5, 2023
@javierm javierm deleted the remove_marker_accessibility branch May 5, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taitus taitus taitus approved these changes

Assignees

@taitus taitus

@javierm javierm

Labels
Accessibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@javierm @taitus