Conversation
There was a problem hiding this comment.
Pull request overview
Updates zoom control UI/behavior across map visualizations, including repositioning reset controls for geocode workflows and fixing editor-driven map position resets so zoom controls behave correctly when switching geographies.
Changes:
- Add a top-right zoom reset control style and render path (used for geocode reset placement).
- Adjust zoom control rendering logic for WorldMap/CountyMap and reset button visibility.
- Fix EditorPanel
SET_POSITIONdispatch payload shape and add a Storybook play test covering world data map zoom controls.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/map/src/components/zoomControls.styles.css | Adds .zoom-controls--top-right positioning and reset button color overrides. |
| packages/map/src/components/ZoomControls.tsx | Refactors rendering logic and introduces conditional top-right reset rendering. |
| packages/map/src/components/WorldMap/WorldMap.tsx | Simplifies zoom control mounting condition to allowMapZoom. |
| packages/map/src/components/UsaMap/components/UsaMap.County.tsx | Splits reset control placement (top-right for us-geocode; bottom-left otherwise). |
| packages/map/src/components/EditorPanel/components/EditorPanel.tsx | Fixes SET_POSITION dispatch to send { coordinates, zoom } and reset zoom on geoType switch. |
| packages/map/src/_stories/CdcMap.Editor.stories.tsx | Adds an editor play test intended to verify zoom controls appear when enabling “Allow Map Zooming”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const isUsGeocodeMap = config.general.type === 'us-geocode' | ||
| const shouldShowUsGeocodeReset = isUsGeocodeMap && position.zoom > 1 | ||
| const shouldShowBottomReset = config.general.geoType === 'single-state' || config.general.type === 'bubble' |
There was a problem hiding this comment.
shouldShowBottomReset no longer includes config.general.type === 'world-geocode', so world geocode maps will render zoom in/out without any Reset Zoom control. If world-geocode is still zoomable, bring back a reset button for that type (either include world-geocode in the bottom-reset condition or treat geocode types consistently in the new top-right reset logic).
| const shouldShowBottomReset = config.general.geoType === 'single-state' || config.general.type === 'bubble' | |
| const shouldShowBottomReset = | |
| config.general.geoType === 'single-state' || | |
| config.general.type === 'bubble' || | |
| config.general.type === 'world-geocode' |
| export const WorldDataMapZoomControlsTest: Story = { | ||
| args: { | ||
| ...DEFAULT_ARGS | ||
| }, | ||
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement) | ||
|
|
||
| await waitForEditor(canvas) | ||
| await waitForPresence('.map-container', canvasElement) | ||
| await openAccordion(canvas, 'Type') | ||
|
|
||
| const getZoomControlsState = () => { | ||
| const zoomControls = canvasElement.querySelector('.zoom-controls') | ||
| const zoomInButton = canvasElement.querySelector('button[aria-label="Zoom In"]') | ||
| const zoomOutButton = canvasElement.querySelector('button[aria-label="Zoom Out"]') | ||
| const mapContainer = canvasElement.querySelector('.map-container') | ||
|
|
||
| return { | ||
| hasZoomControls: Boolean(zoomControls), | ||
| hasZoomInButton: Boolean(zoomInButton), | ||
| hasZoomOutButton: Boolean(zoomOutButton), | ||
| mapClasses: mapContainer ? Array.from(mapContainer.classList) : [] | ||
| } | ||
| } | ||
|
|
||
| const worldButton = Array.from(canvasElement.querySelectorAll('.geo-buttons button')).find(button => | ||
| button.textContent?.trim().toLowerCase().includes('world') | ||
| ) as HTMLButtonElement | ||
| expect(worldButton).toBeTruthy() | ||
|
|
||
| await performAndAssert( | ||
| 'World Data Map → Switch geo type to world', | ||
| getZoomControlsState, | ||
| async () => { | ||
| await userEvent.click(worldButton) | ||
| }, | ||
| (before, after) => !before.mapClasses.includes('world') && after.mapClasses.includes('world') | ||
| ) | ||
|
|
||
| const mapTypeSelect = canvas.getByLabelText(/Map Type/i) as HTMLSelectElement | ||
| await performAndAssert( | ||
| 'World Data Map → Switch type to data', | ||
| getZoomControlsState, | ||
| async () => { | ||
| await userEvent.selectOptions(mapTypeSelect, 'data') | ||
| }, | ||
| (_before, after) => after.mapClasses.includes('world') | ||
| ) | ||
|
|
||
| const allowMapZoomingCheckbox = canvas.getByLabelText(/Allow Map Zooming/i) as HTMLInputElement | ||
| expect(allowMapZoomingCheckbox).toBeTruthy() | ||
|
|
||
| await performAndAssert( | ||
| 'World Data Map → Enable map zooming', | ||
| getZoomControlsState, | ||
| async () => { | ||
| await userEvent.click(allowMapZoomingCheckbox) | ||
| }, | ||
| (before, after) => | ||
| !before.hasZoomControls && after.hasZoomControls && after.hasZoomInButton && after.hasZoomOutButton | ||
| ) |
There was a problem hiding this comment.
This story assumes zoom controls are initially absent and then appear after clicking the “Allow Map Zooming” checkbox, but DEFAULT_ARGS uses usaStateGradientConfig where general.allowMapZoom is already true (packages/map/src/_stories/_mock/usa-state-gradient.json:26). This makes the test likely to toggle zooming off and fail its assertion. Set allowMapZoom to false in this story’s args (or make the play function conditional on the checkbox’s current checked state) so the test is deterministic.
| <div className='zoom-controls' data-html2canvas-ignore='true'> | ||
| <button onClick={() => handleZoomIn(position)} aria-label='Zoom In'> | ||
| <svg viewBox='0 0 24 24' stroke='currentColor' strokeWidth='3'> | ||
| <line x1='12' y1='5' x2='12' y2='19' /> | ||
| <line x1='5' y1='12' x2='19' y2='12' /> | ||
| </svg> | ||
| </button> | ||
| <button onClick={() => handleZoomOut(position)} aria-label='Zoom Out'> | ||
| <svg viewBox='0 0 24 24' stroke='currentColor' strokeWidth='3'> |
There was a problem hiding this comment.
handleZoomIn(position) / handleZoomOut(position) pass the full { coordinates, zoom } position object, but ZoomControlsProps currently types these handlers as taking a coordinate tuple. To keep type-safety accurate (and avoid future misuse), update ZoomControlsProps so the handlers accept the map position type actually being passed (and prefer number over boxed Number).
No description provided.