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-allowed cursor, the aria-disabled label does not toggle to true, 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 pointerEvents style to none might 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 (#1700 (comment)) 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 the this.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 the this.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

• Tooltip

NPM

• ReactDOM v17.0.2

Stack Overflow

• Can I append my component to a div
• Dynamically insert a react component into the dom
• How to wrap content and preserve bound events
• How to inspect a "hover" element

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 title attribute on the span does the heavy lifting here, I haven't observed changes on button.aria-label doing anything useful
• With all that said, we could manipulate span.title like so:
  • When an NC is selected, set span.title to 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.title back to Zoom Out

Resources

• How to display a tooltip on the navigation controls
• Mapbox Docs for map constructor options
• Default Locale config for map constructor

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!