311-data
311-data copied to clipboard
Add Mapbox Zoom Controls to Map Page
Overview
We need to add zoom controls to our map page to improve accessibility and make it clear when user has an NC selected.
Overview
- [x] Add zoom controls that modify Mapbox zoom level (see R1.1, R1.2)
- [x] implement enabled zoom controls (no NC selected)
- [x] implement disabled zoom control (NC is selected)
- [x] implement disabled mouse control
- [x] implement information popver: "Zoom features are disabled while locked into a neighborhood council..." (see figma)
- [x] Establish hard stops on zoom levels for NC selected
- [x] Minimum zoom level: 9
- [x] Maximum zoom level: 15
Resources (R)
- Design Hand-Off Materials 1.1 design ticket: https://github.com/hackforla/311-data/issues/1700 1.2 comment with screenshot of figma notes 1.3 Comment with screenshot of zoom controls (final) 1.4 Figma Link
- Mapbox Docs links 2.1 Docs example of setting zoom levels 2.2 Navigation Control Docs 2.3 Custom styles for mapbox controls
- UX Research's October '23 usability test 3.1 Slide indicating users required a zoom in/out feature
PM Review Section
Screenshot before proposed changes
TODO: Screenshot after proposed changes
[insert screenshot here]
Resolved Dependency Info
Dependency (Resolved)
- [x] Need to confirm with design team NC selected zoom behavior
- don't disable zoom controls when NC is selected
- prevent users from zooming too far out when NC is selected
- [x] Need to establish zoom levels for NC selected
- [x] Minimum zoom level
- [x] Maximum zoom level
This ticket is ready to be picked up
Assigning this to @DrAcula27 based on conversation from earlier in the day.
@DrAcula27 please provide the following information:
- ETA
- availability for communications while working on the ticket.
Thank you!
ETA: 10 September Availability: weekdays after 2pm Pacific time
Updated ETA: 17 September
- due to working on large issue for website team
Availability: weekdays after 2pm Pacific time
Finally finished the large issue for the website team 🚀
- new ETA: end of the week
@ryanfchase As mentioned in the meeting tonight, I'd like to schedule a quick meetup with someone to walk through the codebase, so I can more easily tackle this issue.
Availability:
- Tomorrow (26 September 2024) from 0900 to 1200 Pacific Time
- Friday (27 September 2024) from 0900 to 1600 Pacific Time
- Next week:
- Weekdays from 1400 to 1900 Pacific Time
ETA: October 11 with a check-in on the 2nd via either GitHub or Slack.
Help Wanted
@ryanfchase @traycn
- View my branch here: https://github.com/DrAcula27/311-data/tree/add-mapbox-zoom-1793
Questions
- When disabling the zoom controls after a user selects a NC, the cursor does not change to the
not-allowedcursor, thearia-disabledlabel does not toggle totrue, and no tooltip appears when I hover over the zoom controls. The code I am using is:
// grey out and disable the zoom buttons
const zoomInButton = document.querySelector(
'.mapboxgl-ctrl-zoom-in'
);
const zoomOutButton = document.querySelector(
'.mapboxgl-ctrl-zoom-out'
);
if (zoomInButton && zoomOutButton) {
zoomInButton.style.opacity = '0.5';
zoomInButton.style.pointerEvents = 'none';
zoomInButton.style.cursor = 'not-allowed';
zoomInButton.setAttribute('aria-disabled', 'true');
zoomOutButton.style.opacity = '0.5';
zoomOutButton.style.pointerEvents = 'none';
zoomOutButton.style.cursor = 'not-allowed';
zoomOutButton.setAttribute('aria-disabled', 'true');
}
What am I missing here?
Some thoughts are, changing the
pointerEventsstyle tononemight be messing with things, Mapbox versioning issues, and styles overriding each other.
- Once I get this working, how can I use the
getZoom()Mapbox method to allow the user to zoom between max allowed zoom and the zoom level the map goes to when a NC is selected?
@DrAcula27 I think that setting pointer events to none was definitely preventing the cursor styles from being applied. I actually removed both these lines (from their respective blocks):
zoomInButton.style.pointerEvents = 'none'; // remove this
zoomInButton.style.cursor = 'not-allowed'; // remove this
I then simply zoomed in until I hit the max zoom level, and the pointer: not-allowed and aria-disabled="true" were applied automatically without needing custom CSS. I guess Mapbox simply wants control over these rules.
I've done some digging into programmatically setting min + max zooms. I cannot find any documentation on this, HOWEVER, you can simply call this.map.setMinZoom(z1) and this.map.setMaxZoom(z2).
You'll notice that when activating an NC, we frame the camera around that NC's bounding box. To get this exact zoom level, I think you can call this.map.getZoom(), and then plug that into setMinZoom(), perhaps with a few fractional zoom levels of wiggle room (up to you, we can review when we demo this).
Update: I worry there is some funkiness of grabbing this.map.getZoom right when we ask the map to zoom in on an NC. I think we would need to look into this.map.once('zoomend', () => ...) to ensure we grab the desired zoom level.
Another approach would be to prevent panning beyond the NC's bounding box. To do this, you can call boundingBox(geo), provided from geoUtils.js. I'm not sure off the top of my head where to obtain the geo feature, but I assume that you'll have access to the NC's geo object when you select it. I'm fairly certain geo, in this context, means a Feature, taking on the geojson shape as an object.
Resources
A single geojson Feature:
{
"type": "Feature",
"geometry": {
"type": "Point",
"coordinates": [125.6, 10.1]
},
"properties": {
"name": "Dinagat Islands"
}
}
Source on setting max bounds: https://stackoverflow.com/questions/61523571/how-to-fit-bounds-and-set-max-bounds-at-same-time-on-window-resize-in-mapbox
Where to find all NC geojson: data/ncGeojson.js
I've just remembered that in the design ticket (https://github.com/hackforla/311-data/issues/1700#issuecomment-2311594526) we specified to use a max zoom of 18, and a min zoom of 9 when an NC is selected. PMs will want to check this behavior and determine if it is good enough. To avoid complexity mentioned above, feel free to move forward with these hard-coded zoom values before worrying about NC bounding boxes or pan areas.
Update to tooltip text:
- From:
Zoom features are disabled while locked into a neighborhood council. To enable zoom features, please exit by clicking out of the selected area. - To:
Zoom features are limited while locked into a neighborhood council. To reset zoom features, please exit by clicking out of the selected area. - Use same styles as the info popovers in the filter box, and demo next week to design team.
Observation:
- When setting
this.map.setMinZoom(<zoomLevel>)after thethis.map.once('zoomend', () => ...)code block, testing revealed that the screen whites out for about a second when selecting a NC. When deselecting, this white-out does not happen. - White-out issue resolves by moving the
this.map.setMinZoom(<zoomLevel>)to inside thethis.map.once('zoomend', () => ...)code block.
Observation:
- If a user double-clicks on a NC, the zoom only goes in one zoom level instead of zooming into and centering on the selected NC.
ETA Update:
- 16 October 2024 after the meeting
- Availability: weekdays after 2pm pacific
- Most of the functionality is complete, will need to demo design/style choices at the meeting before PR.
In the PR, #1843, I suggested a different approach. I'll list in more detail how to implement my suggestion:
On map load, we'd like to wrap a zoom control with the Tooltip component. This lets us reuse styles that we've utilized for the Datepicker tooltip, etc.
- e.g., consider the DOM for our zoom out button:
<button class="mapboxgl-ctrl-zoom-out" type="button" aria-label="Zoom Out" aria-disabled="false"> <span class="mapboxgl-ctrl-icon" aria-hidden="true" title="Zoom Out"></span> <!-- hint: wrap this one! --> </button> - you may consider doing this after an NC is selected (as opposed to on map load), but I slightly prefer adding the Tooltip on map load, and figuring out a way to keep the tooltip inactive until that point
- Please experiment with using RenderDOM.render to place the tooltip onto the DOM, to my knowledge, it inserts the component (1st arg) as a child component on to the DOM element (2nd arg)
- check out the resources below for examples
- I found "how to wrap content and preserve bound events" the most useful for wrapping Tooltip around the span
Resources
MUI Docs
NPM
Stack Overflow
Also, I have another approach which is slightly simpler. It is detailed below, but note that you cannot utilize any of the styles defined in our React components. We'll need to significantly modify the CSS to match the specs.
- As seen in the stack overflow post, you can override the locale config to use custom tooltips on navigation controls
new mapboxgl.Map({ // ... the usual options ... locale: { 'NavigationControl.ZoomIn: 'my-custom-tooltip-text', } }) - If you play around this setting, you'll see that setting custom tooltips for zoom-in / zoom-out results in the following DOM:
<button class="mapboxgl-ctrl-zoom-out" type="button" aria-label="my-custom-tooltip-text" aria-disabled="false"> <span class="mapboxgl-ctrl-icon" aria-hidden="true" title="my-custom-tooltip-text"></span> <!-- here! --> </button>- The
titleattribute on the span does the heavy lifting here, I haven't observed changes onbutton.aria-labeldoing anything useful
- The
- With all that said, we could manipulate
span.titlelike so:- When an NC is selected, set
span.titleto be: (this is the desired text we agreed upon earlier)
Zoom features are limited while locked into a neighborhood council. To reset zoom features, please exit by clicking out of the selected area.- When the NC is de-selected, set
span.titleback toZoom Out
- When an NC is selected, set
Resources
Updated ETA
- 30 October 2024
@ryanfchase I've got the tooltip implemented, but with default styles. Let me know if I should make the PR as-is, or if I should work on implementing style changes.
Proposed styles according to Figma
- Background color of #29404f
- White text
- Fully opaque
- No arrow
Current default styles from MUI
- Background color of a dark grey
- White text
- Somewhat transparent
- Arrow
@DrAcula27 let's give design a little more time to review and officially hand off their work. In the meantime, please prepare all of your work in your PR and prepare for official review. I think you've done a lot of good work, I'd like to get these changes into the codebase before our Vite migration on Wednesday (10/30). The code for styling for this design will come in a separate ticket, most likely.
For updating the styles of the tooltip: https://mui.com/material-ui/react-tooltip/#customization
I've provided my review. @DrAcula27 please move this ticket to In Review when you are ready for re-review. Thanks, and good work!