-
Notifications
You must be signed in to change notification settings - Fork 128
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
VACMS-18435 Facility locator markers to numbers #32021
Conversation
@@ -1,18 +0,0 @@ | |||
import DivMarker from './DivMarker'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is not used or referenced anywhere. It was added to the repo ~4 years ago and the file that referenced it (VAMap.jsx) was deleted shortly after.
a554f6e
to
f7428c4
Compare
4955a10
to
1c9c9a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with it, but if they change base font size, the marker will change. Does that potentially lead to problems later? Like markers taking too much space? Does text zoom change the marker size a lot (is it the same marker as on the map)? Waiting on RI because I have something building locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@eselkin I have some screenshots in the description that show the markers at "very large" text size in Chrome. They do get pretty big, but they're not so large that they cause problems for the other parts of the UI, IMO. |
@eselkin and @randimays I'm still waiting for the RI as well but FWIW, before I wrote up this ticket and the testing steps I specifically looked at the sizing of the markers and if I wanted them to scale or not. I ended up at the same conclusion as Randi and thought the benefits of the resizing (ability to see them better, tap them better, etc. - which are the reasons a user might be increasing their font size to begin with) outweighed the possible con of taking up more space on the map. |
Sorry all; the review instance was working when I requested your reviews. Seems it just died. Same thing happened on Chris's PR as well. Looking into it... |
@laflannery @eselkin I still can't get the RI for this PR to work. But! I have this commit included in another Facility Locator PR, and its review instance is working: http://af26f390210ec3fc693f3447480e41fe.review.vetsgov-internal/find-locations If you want to test in that review instance, here are the tickets you can test in it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving for all 3 tickets mentioned in this comment
@thejordanwood Forgot to tag you in this comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the milage and the number tickets! I don't really need to approve the semantic tag ticket.
Are you removing, renaming or moving a folder in this PR?
Did you change site-wide styles, platform utilities or other infrastructure?
Summary
Convert the markers in Facility Locator from letters to numbers. Includes updates to styling to make the markers flexible for different zoom levels / browser font sizes.
Also includes an update to move the distance / mileage below the facility name.
Figma for distance / mileage movement: https://www.figma.com/design/GvNVeG9GAXZaBpny7vNxZP/Facility-Locator?node-id=745-13219&m=dev
Related issue(s)
Testing done
Tested all Facility Types on desktop and mobile including different browser font sizes.
Screenshots
Mobile
Medium font size
Vet centers / VA cemeteries
VA benefits / Community pharmacies
Community providers / Emergency care
Urgent care / VA health
Other font sizes (List view)
Other font sizes (Map view)
Desktop
Medium font size
Vet centers
VA cemeteries
VA benefits
Community pharmacies
Community providers
Emergency care
Urgent care
VA health
Other font sizes
What areas of the site does it impact?
Facility locator only.
Acceptance criteria
Quality Assurance & Testing