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

Add Mapbox Zoom Controls to Map Page #1793

Open
11 of 12 tasks
ryanfchase opened this issue Jul 1, 2024 · 17 comments · May be fixed by #1843
Open
11 of 12 tasks

Add Mapbox Zoom Controls to Map Page #1793

ryanfchase opened this issue Jul 1, 2024 · 17 comments · May be fixed by #1843
Assignees
Labels
Complexity: Medium P-feature: Map Role: Frontend React front end work Size: 2pt Can be done in 7-12 hours

Comments

@ryanfchase
Copy link
Member

ryanfchase commented Jul 1, 2024

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

  • Add zoom controls that modify Mapbox zoom level (see R1.1, R1.2)
    • implement enabled zoom controls (no NC selected)
    • implement disabled zoom control (NC is selected)
      • implement disabled mouse control
      • implement information popver: "Zoom features are disabled while locked into a neighborhood council..." (see figma)
  • Establish hard stops on zoom levels for NC selected
    • Minimum zoom level: 9
    • Maximum zoom level: 15

Resources (R)

  1. Design Hand-Off Materials
    1.1 design ticket: Improve Zoom Behavior on Map #1700
    1.2 comment with screenshot of figma notes
    1.3 Comment with screenshot of zoom controls (final)
    1.4 Figma Link
  2. Mapbox Docs links
    2.1 Docs example of setting zoom levels
    2.2 Navigation Control Docs
    2.3 Custom styles for mapbox controls
  3. 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

image

TODO: Screenshot after proposed changes

[insert screenshot here]

Resolved Dependency Info

Dependency (Resolved)

  • 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
  • Need to establish zoom levels for NC selected
    • Minimum zoom level
    • Maximum zoom level

@ryanfchase ryanfchase self-assigned this Jul 1, 2024
@ryanfchase ryanfchase added Role: Frontend React front end work Size: Missing P-feature: Map Dependency An issue that includes dependencies labels Jul 1, 2024
@ryanfchase ryanfchase added Size: 2pt Can be done in 7-12 hours and removed Dependency An issue that includes dependencies Size: Missing labels Aug 28, 2024
@ryanfchase ryanfchase removed their assignment Aug 28, 2024
@ryanfchase
Copy link
Member Author

This ticket is ready to be picked up

@ryanfchase
Copy link
Member Author

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!

@DrAcula27
Copy link
Member

ETA: 10 September
Availability: weekdays after 2pm Pacific time

@DrAcula27
Copy link
Member

Updated ETA: 17 September

  • due to working on large issue for website team

Availability: weekdays after 2pm Pacific time

@DrAcula27
Copy link
Member

Finally finished the large issue for the website team 🚀

  • new ETA: end of the week

@DrAcula27
Copy link
Member

DrAcula27 commented Sep 26, 2024

@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

@DrAcula27
Copy link
Member

ETA: October 11 with a check-in on the 2nd via either GitHub or Slack.

@DrAcula27 DrAcula27 added the Help Wanted Extra attention is needed label Oct 8, 2024
@DrAcula27
Copy link
Member

Help Wanted

@ryanfchase @traycn

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?

@ryanfchase
Copy link
Member Author

ryanfchase commented Oct 9, 2024

@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

@ryanfchase
Copy link
Member Author

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.

@DrAcula27
Copy link
Member

DrAcula27 commented Oct 10, 2024

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.

@DrAcula27
Copy link
Member

DrAcula27 commented Oct 10, 2024

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.

@DrAcula27
Copy link
Member

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.

@DrAcula27 DrAcula27 removed the Help Wanted Extra attention is needed label Oct 10, 2024
@DrAcula27
Copy link
Member

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.

@DrAcula27 DrAcula27 linked a pull request Oct 16, 2024 that will close this issue
4 tasks
@ryanfchase
Copy link
Member Author

ryanfchase commented Oct 17, 2024

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

@ryanfchase
Copy link
Member Author

ryanfchase commented Oct 17, 2024

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

@DrAcula27
Copy link
Member

Updated ETA

  • 30 October 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Medium P-feature: Map Role: Frontend React front end work Size: 2pt Can be done in 7-12 hours
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

2 participants