feat[mobile]: swipe back gesture#2572
Conversation
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@js/app/packages/app/component/split-layout/components/PopoverSplitRenderer.tsx`:
- Around line 76-77: The popover stub in PopoverSplitRenderer currently returns
an empty array from the history() stub which hides the current entry; change the
history implementation on the SplitHandle stub (the history() method) to return
the current popover content (e.g., [props.popover.content]) instead of [] while
leaving previousContent() as () => null so the handle correctly reports "up to
and including the current one."
In
`@js/app/packages/app/component/split-layout/mobile/createMobileSwipeLayout.ts`:
- Around line 44-55: The mobile swipe layout seeds slotA from
splitManager.splits()[0] which can be the wrong foreground; instead initialize
slotASplitId and slotBSplitId using splitManager.activeSplitId() and the visible
non-background splits (or a normalized split list) so the true active split
becomes FG; in createMobileSwipeLayout find activeId =
splitManager.activeSplitId(), set slotASplitId to activeId (or flip fgIsSlotA
appropriately), set slotBSplitId to the first other visible/non-background split
(or undefined if none), and ensure fgIsSlotA/toggleFgSlot logic remains
consistent with those initial assignments so both visible splits are attached.
- Around line 124-145: The batch in completeSwipeBack currently calls
splitManager.removeSplit(currentFgId) and
splitManager.setBackground(currentBgId, false) but does not activate the
promoted split, leaving activeSplitId pointing to the removed FG; within the
same batch after setBackground (and before toggleFgSlot()/setNewBgSlotId), call
splitManager.activateSplit(currentBgId) so the manager's activeSplitId is
updated immediately; update the sequence around removeSplit, setBackground,
activateSplit, then createNewSplit/setNewBgSlotId/toggleFgSlot to ensure readers
of activeSplit() see the promoted FG.
In `@js/app/packages/app/component/split-layout/SplitLayout.tsx`:
- Around line 316-323: The URL-sync logic only reacts to split count via
createLayoutUrlSync and misses FG/BG swaps from the mobile override; update the
sync to watch the visible URL computed from splitManager.getUrlSegments() and
navigate whenever it differs from the current pairs() path. Specifically, add a
reactive effect (e.g., createEffect with a visibleUrl() =
splitManager.getUrlSegments().join('/')) that compares the new visibleUrl to
pairs().join('/') and calls navigate(`/${visibleUrl}`) when they differ, and
ensure you clean up the effect and keep the existing mobileSwipeLayout
override/unset behavior around splitManager.setMobileForwardOverride (so
navigateForward/completeSwipeBack-driven URL changes are reflected).
- Around line 600-609: The nested ternary inside fgStyle() should be moved into
a new helper named getFgTransition(): inspect isDragging() and isAnimatingOut()
and return 'none' when dragging, `transform ${SWIPE_ANIMATION_MS}ms ease-out`
when animating out, otherwise 'none'; then replace the transition expression in
fgStyle() with a call to getFgTransition(). Keep existing references to
dragOffset(), isDragging(), isAnimatingOut(), and SWIPE_ANIMATION_MS so behavior
remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0c76b35-1695-4825-93d8-e0a5f5ab56b4
📒 Files selected for processing (7)
js/app/packages/app/component/mobile/MobileDock.tsxjs/app/packages/app/component/split-layout/SplitLayout.tsxjs/app/packages/app/component/split-layout/components/PopoverSplitRenderer.tsxjs/app/packages/app/component/split-layout/components/SplitContainer.tsxjs/app/packages/app/component/split-layout/layout.tsjs/app/packages/app/component/split-layout/layoutManager.tsjs/app/packages/app/component/split-layout/mobile/createMobileSwipeLayout.ts
We use our split layout system to manage mobile swipe back behavior. We always keep two splits mounted: the current "foreground" split, and the previous "background" split, which is revealed if the user swipes from the left side of the screen.