Implement spy mode#1651
Merged
Merged
Conversation
…lization of comparison modes
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two bugs prevented popover toolbar buttons from showing in the generic viewer layout template: 1. Z-index timing gap: react-tiny-popover applies containerStyle via useEffect (after browser paint), so the portal container was briefly at z-index:auto before the first paint. For positioned elements in the same stacking context, z-index:auto stacks below elements with explicit positive z-index (e.g. the toolbar group at z-index:10). Switched from containerStyle to containerClassName + a CSS rule, which applies z-index synchronously at element-creation time via useState initialiser in react-tiny-popover's useElementRef. 2. focus() stealing in onMouseDown: spy mode added tabIndex+focus() to map-viewer-component so keyboard ArrowUp/Down resize the spy cursor. That focus() was called for every mousedown inside the component, including clicks on toolbar popover-trigger buttons, stealing focus before the click event completed and potentially disrupting the click-outside detection cycle. Fixed by only calling focus() when the mousedown target is not a button/input/interactive element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r height react-tiny-popover uses document.body as the default boundary element when positioning popovers. In generic.html, html and body had no explicit height, so getBoundingClientRect() returned height=0 / bottom=0. This caused react-tiny-popover's nudging algorithm to force the popover top to (0 - popoverHeight = -186px), placing it entirely above the viewport. Adding height: 100% to html, body, and #map ensures the body bounding rect covers the full viewport, so popover boundary calculations are correct. Diagnosed via Playwright: scoutRect.top was 0, but the boundary bottom was also 0, causing finalTop = popoverHeight * -1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ViewerOptions placeholder renders its own content without a background. Unlike the map switcher popover which already wraps content in a Card, the viewer options popover was passing the placeholder directly, resulting in a transparent background that showed the map through the popover content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…p, ESC to close
Swipe mode:
- Primary/secondary labels now center-aligned within their respective halves
of the map (instead of left/right edges), positioned relative to swipe divider
Spy mode:
- Removed static primary/secondary labels and Close Comparison button
- Added a tracking tooltip that follows the spy cursor, positioned outside
the spy circle (to its right), showing primary/secondary map names and
an ESC-to-close hint
- ESC key now closes comparison via a document-level keydown listener,
so it works without requiring map focus; ArrowUp/Down resize still
require map focus
- Added MAP_SPY_ESC_HINT string ("Press ESC to close")
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… in one flex row Labels now live in a single absolutely-positioned flex row centered on the divider line, so Primary and Secondary sit at a fixed gap directly beside the Close Comparison button regardless of where the splitter is. Removed the ComparisonLabels helper component (no longer needed) and extracted a SWIPE_LABEL_STYLE constant for the shared badge styling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace ⬤/◯ icon prefixes with "Primary:" / "Secondary:" text labels, showing the map's display name (primaryLabel/secondaryLabel if set, falling back to primaryMapName/secondaryMapName). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Bundle Size Report ✅ PASS
Top Changes
Generated by build-size-diff Commit: 27b65bb |
|
coverage: 60.54% (-0.3%) from 60.795% — feature/spy-mode into master |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors and generalizes the map comparison feature, moving from a swipe-specific implementation to a more extensible model that supports multiple comparison modes (such as swipe and spy). The changes update type definitions, actions, and parsing logic to use the new terminology and data structures, making it easier to add or manage new comparison renderers in the future.
Generalization of map comparison:
ComparisonModetype ("none" | "swipe" | "spy") and a generalizedIComparisonPairinterface to replace the old swipe-specificIMapSwipePair, with type aliases for backward compatibility. ([[1]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-62eba76fe6cadaa56e99795f9b9fd052fa75b11f8d2a22bdcb0519eb43f723dcL1893-R1906),[[2]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-62eba76fe6cadaa56e99795f9b9fd052fa75b11f8d2a22bdcb0519eb43f723dcR1938-R1939))comparisonPairsandcomparisonMode, with compatibility fields formapSwipePairsandswipeActive. Added new state fields forlastComparisonModeandspyCursorRadius. ([[1]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-62eba76fe6cadaa56e99795f9b9fd052fa75b11f8d2a22bdcb0519eb43f723dcL2113-R2152),[[2]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-62eba76fe6cadaa56e99795f9b9fd052fa75b11f8d2a22bdcb0519eb43f723dcR2161-R2167), [3]Action and reducer updates:
ISetComparisonModeAction,ISetSwipePositionAction, andISetSpyCursorRadiusAction. Updated action creators accordingly and provided compatibility aliases. ([[1]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-82b4c79ae83862ba64e8edb0ab72b2dbcf311d479ed187addc28af83d23bbb63L997-R1030),[[2]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-82b4c79ae83862ba64e8edb0ab72b2dbcf311d479ed187addc28af83d23bbb63L1083-R1098),[[3]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-cb3f3831b3c8a6045b000d641abc24af41f44da38c58f7734d3024b750a94d30L50-R52),[[4]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-cb3f3831b3c8a6045b000d641abc24af41f44da38c58f7734d3024b750a94d30L966-R1027))Application definition and parsing:
Comparison*keys (e.g.,ComparisonPairWith,ComparisonPrimary) instead ofSwipe*keys, and updated the parsing function toparseComparisonPairs. ([[1]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b1721891163f79a2e27b36e56c5313d7c40c3112f719b0ff4183861fd1d90e9bL10-R18),[[2]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b1721891163f79a2e27b36e56c5313d7c40c3112f719b0ff4183861fd1d90e9bL28-R39),[[3]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b1721891163f79a2e27b36e56c5313d7c40c3112f719b0ff4183861fd1d90e9bR49-R50),[[4]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b149e4949d36eb0bea0deef3b28dfcfee7617191327fb008f2a546f95c433315L15-R15),[[5]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b149e4949d36eb0bea0deef3b28dfcfee7617191327fb008f2a546f95c433315R147),[[6]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-b149e4949d36eb0bea0deef3b28dfcfee7617191327fb008f2a546f95c433315L168-R170))Documentation and context:
CONTEXT.mdto define the new comparison terminology and relationships, clarifying the distinction between comparison pairs and rendering modes. Added an ADR documenting the motivation for this generalization. ([[1]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-86fee53c33308768d49a4ac0ff5720fd7f5411a8c8bd62ab9b80bbffa2977a14L3-R3),[[2]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-86fee53c33308768d49a4ac0ff5720fd7f5411a8c8bd62ab9b80bbffa2977a14R23-R71),[[3]](https://github.com/jumpinjackie/mapguide-react-layout/pull/1651/files#diff-0f1b984675062aec948730970267fbd15184ce5a634ecd07213085c35da184dbR1-R3))These changes lay the groundwork for supporting multiple map comparison renderers in the viewer and clarify the internal and external APIs around map comparison.
Fixes #1650