Conversation
…-to animation - Collection view now fills full container height by collapsing axis labels bar to 0 height (max-height/padding/border-color CSS transitions) - Grouped view animates axis labels at bottom with a 600ms fade-in + expand effect - Trackpad pinch-to-zoom uses normalized delta factor (0.003 for pixel-mode, 0.05 for line-mode) for consistent feel - Selecting an event now uses animateZoomAndScroll in both collection and grouped view modes for a smooth 'fly to' animation that centers the card Co-authored-by: einari <134365+einari@users.noreply.github.com>
|
@copilot Great. The pinch to zoom can be a bit "faster" - greater steps. Get the same feeling as an infinite canvas Miro type of application. Also, as you can see in the GIF, when I'm in for instance Group mode and click an event, it doesn't "move the camera" - scroll to it as I would expect. Remember, we should aim at keeping the selected card in center at all times. |
- Increase wheel zoom factors for Miro-like responsiveness: - DOM_DELTA_PIXEL (trackpad): 0.003 → 0.01 (~3x faster) - DOM_DELTA_LINE (mouse wheel): 0.05 → 0.12 (~2.5x faster) - Fix grouped-view card selection: both useCardSelection and useDetailPanelClose were computing the scroll container incorrectly (finding .pv-main parent instead of the actual scrollable .pv-viewport). Pass containerRef directly to both hooks and use containerRef.current as the scroll target. Co-authored-by: einari <134365+einari@users.noreply.github.com>
…range Co-authored-by: einari <134365+einari@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves PivotViewer interaction fidelity by making zoom gestures feel more responsive and fixing “fly-to” camera animations in grouped mode by using the actual scrollable viewport and a real spacer element during animations.
Changes:
- Normalize wheel/pinch zoom deltas by
deltaModeto speed up trackpad pinch and mouse-wheel zoom. - Fix grouped-mode fly-to by passing
containerRef(scrollable.pv-viewport) andspacerRefdirectly into selection/close hooks instead of relying on DOM traversal/querying. - Update axis label styling to fade and collapse when switching views.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Source/PivotViewer/utils/selection.ts | Uses animateZoomAndScroll to animate scroll-to-center (including collection mode / subsequent selections). |
| Source/PivotViewer/hooks/useWheelZoom.ts | Adjusts zoom delta scaling based on WheelEvent.deltaMode for snappier pinch/wheel zoom. |
| Source/PivotViewer/hooks/useDetailPanelClose.ts | Switches to containerRef.current so grouped-mode close can scroll/animate the correct viewport. |
| Source/PivotViewer/hooks/useCardSelection.ts | Switches to containerRef.current and spacerRef.current to ensure fly-to uses real scroll container + spacer. |
| Source/PivotViewer/PivotViewer.tsx | Threads containerRef/spacerRef into the updated hooks. |
| Source/PivotViewer/PivotViewer.css | Collapses axis-label area (max-height/padding/border) in addition to opacity fade. |
Comments suppressed due to low confidence (2)
Source/PivotViewer/utils/selection.ts:117
- In collection mode this path uses
animateZoomAndScrollwithstartZoom === targetZoom, which still runs a full rAF animation loop and callssetZoomLevelon every frame even though the zoom never changes. To reduce unnecessary work/state scheduling, consider usingsmoothScrollTohere (as before) or adjusting the animation helper so it can animate scroll-only without repeatedly invokingonUpdatewhen zoom is unchanged.
if (viewMode === 'collection') {
// Collection mode: animate scroll to center the selected card (no zoom change)
if (cardPosition) {
setIsZooming(true);
const { scrollLeft, scrollTop } = calculateCenterScrollPosition(container, cardPosition, zoomLevel, 0, totalHeight);
animateZoomAndScroll({
container,
cardPosition,
startZoom: zoomLevel,
targetZoom: zoomLevel,
targetScrollLeft: scrollLeft,
targetScrollTop: scrollTop,
onUpdate: setZoomLevel,
onComplete: () => setIsZooming(false),
});
Source/PivotViewer/utils/selection.ts:151
- For subsequent selections,
animateZoomAndScrollis also used withstartZoom === targetZoom(scroll-only), which means the helper still schedules rAF frames and callssetZoomLeveleach frame despite no zoom delta. Consider switching this tosmoothScrollTo(or a scroll-only animation path) to avoid extra per-frame state work.
// Subsequent selections: animate the scroll to center the new card
if (cardPosition) {
setIsZooming(true);
const detailWidth = viewMode === 'collection' ? 0 : DETAIL_PANEL_WIDTH;
const { scrollLeft, scrollTop } = calculateCenterScrollPosition(container, cardPosition, zoomLevel, detailWidth, totalHeight);
animateZoomAndScroll({
container,
cardPosition,
startZoom: zoomLevel,
targetZoom: zoomLevel,
targetScrollLeft: scrollLeft,
targetScrollTop: scrollTop,
onUpdate: setZoomLevel,
onComplete: () => setIsZooming(false),
});
| // Use the containerRef directly as the scrollable viewport | ||
| const container = containerRef.current; | ||
| if (!container || !selectedItem) { | ||
| setSelectedItem(null); | ||
| return; |
There was a problem hiding this comment.
useDetailPanelClose now correctly uses containerRef.current, but the zoom-out animation later calls animateZoomAndScroll without providing getLayoutSizeAtZoom (available from createCardPositionCallbacks) and without a spacer element to resize during the animation. This can leave scrollWidth/scrollHeight out of sync with the animated zoom, causing clamped/jumpy scroll during the fly-to. Consider passing getLayoutSizeAtZoom: callbacks.getLayoutSizeAtZoom and threading a spacerRef into this hook (similar to useCardSelection) so the spacer can be updated synchronously during the animation.

Addresses two UX issues raised in review: trackpad pinch zoom was too sluggish, and clicking a card in grouped mode failed to animate the camera to it.
Fixed
Grouped-mode fly-to broken: Both
useCardSelectionanduseDetailPanelCloseresolved the scroll container via DOM traversal (.pv-main > parentElement), which returned the outer.pivot-viewerwrapper — a non-scrollable element. All scroll reads/writes were silently no-ops. Both hooks now receivecontainerRefdirectly and usecontainerRef.current(the actual.pv-viewportscrollable element).Spacer always null during zoom animation:
useCardSelectionwas finding the animation spacer viacontainer.querySelector('.pv-spacer')— no element with that class exists, so the spacer was alwaysnull. This caused incorrect scroll-range clamping during zoom animations. Hook now acceptsspacerRefand usesspacerRef.currentdirectly.Pinch zoom too slow: Trackpad pixel-mode factor increased
0.003 → 0.01; mouse-wheel line-mode factor increased0.05 → 0.12for a snappier infinite-canvas feel.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.