Added swipe animations between chapters on PWA #230#988
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdated ChangesDrag-driven chapter navigation
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/lib/components/ChapterSelector.svelteOops! Something went wrong! :( ESLint: 9.39.4 The requested operation requires ESLint to serialize configuration data, Please double-check your configuration for errors. If you still have problems, please stop by https://eslint.org/chat/help to chat src/routes/text/+page.svelteOops! Something went wrong! :( ESLint: 9.39.4 The requested operation requires ESLint to serialize configuration data, Please double-check your configuration for errors. If you still have problems, please stop by https://eslint.org/chat/help to chat 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 |
Awest882
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/text/`+page.svelte:
- Around line 158-163: handleMouseDown currently only starts dragging when
swipeBetweenBooks is true, which prevents same-book chapter swipes; change the
gating so dragging starts when either cross-book swiping is enabled or
intra-book swiping is possible. Update the if condition in handleMouseDown to
check a combined predicate (e.g., swipeBetweenBooks || canSwipeChapters or
swipeBetweenBooks || totalChapters > 1) instead of just swipeBetweenBooks, and
ensure isDragging and startX (using x.current) are set under that combined
condition.
- Around line 708-710: Remove the aria-hidden="true" attribute from the active
reading container element (the element with class "max-w-screen-md mx-auto" and
onpointerdown={handleMouseDown}) so the primary chapter content is exposed to
assistive technologies; locate that element in src/routes/text/+page.svelte and
delete the aria-hidden attribute (and any conditional logic that sets it) or
replace it with appropriate aria attributes if necessary for interaction,
ensuring handleMouseDown remains attached.
- Around line 141-156: handleMouseUp currently runs on any pointer-up and can
trigger unwanted navigation and race conditions; first check the active drag
flag (isDragging) and return immediately if not dragging, then set isDragging =
false only after handling; when deciding direction use Math.abs(x.current) <
minSlideDistance() as before but before calling
navigateToTextChapterInDirection(...) verify navigation eligibility (e.g. ensure
prev/next is available or call a canNavigate(direction) check) and call await
navigateToTextChapterInDirection(direction) so navigation completes before
resetting the animated x store; finally reset x with x.set(0, { duration: 0 })
only after the awaited navigation and keep references to draggableWidth and
minSlideDistance() as used in handleMouseUp and x to preserve existing behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8a1619e-1087-4110-9be2-61c948424f22
📒 Files selected for processing (1)
src/routes/text/+page.svelte
Awest882
force-pushed
the
feature/swipe-animation/230
branch
2 times, most recently
from
7bd04ac to
0e12659
Compare
| let startX = 0; | ||
| let isDragging = $state(false); | ||
| let draggableWidth = $state(0); | ||
| let minSlideDistance = () => draggableWidth / 3; // use to determine how far a user has to slide to move to the next chapter |
There was a problem hiding this comment.
This is a bit confusing for the user. The watershed for which chapter to "fall" to should be at the midpoint between the chapters, not 33% percent of the way between them (see screen recording). However, the user also shouldn't need to swipe very far to trigger the transition.
I suggest using a Spring instead, because it supports momentum - if the user releases early it could be set up to still trigger the transition, while the watershed can be set to 50% like it is on the myk.global.bible mobile version Chris mentioned.
There was a problem hiding this comment.
Maybe could use two springs - one until the transition midpoint and one after - the first moves toward the current chapter while the second moves toward the next chapter. Then the chapter navigation could be based on the current horizontal position of the windows, not the user's mouse or finger being released (i.e. the reference change would trigger only if the end of the horizontal motion was reached).
|
To the recent comment, I just pulled in the changes from last night changing from Program Files (x86) to Program Files. Nothing should have changed for that. |
|
Another thing I noticed was that borders aren't being handled properly. They aren't being shown at all (a different, existing issue), but the swipe animation doesn't involve the (blank) borders. Here's the Android version for comparison. The Android version also has its watershed set to maybe 60% of the way to the next chapter. Screen.Recording.2026-06-15.084823.mp4Screen.Recording.2026-06-15.085204.mp4 |
* Horizontal swiping motion * Text for previous/next chapters will slide into the place of the current chapter * Swipe handler activates when user swipes 1/3 of the text width (Adjustments can be made to `minSlideDistance`)
One div was closed in the wrong place
* Allow swipes only in the direction a valid chapter is in
* Hiding overflow on x axis * Keeps text centered
* No longer able to see chapters you cannot swipe to * Reduced loading overhead * Fixed selector navigation bug
Awest882
force-pushed
the
feature/swipe-animation/230
branch
from
0e12659 to
5ec5544
Compare
FyreByrd
left a comment
FyreByrd
left a comment
There was a problem hiding this comment.
Switching chapters in the chapter selector is not triggering the animation (is it supposed to?). Also, switching chapters in the book selector should probably have the same behavior.
Text selection is also broken by this.
| } | ||
|
|
||
|
|
||
| async function handleMouseUp(_event: any) { |
There was a problem hiding this comment.
Why not type this as PointerEvent? Same comment for other handlers.
There was a problem hiding this comment.
I didn't know the type that svelte used for those events. It's been changed
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/text/+page.svelte (1)
124-132:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh
settingsCacheafter non-animated swipe navigation.The page now renders from
settingsCache, butdoSwipe()still only updates$refs. On story pages, swiping the illustration can navigate history/refs while the text panels keep rendering the previous cached references.Suggested fix
- await navigateToTextChapterInDirection(swipeDirection === 'right' ? -1 : 1); + await navigateToTextChapterInDirection(swipeDirection === 'right' ? -1 : 1); + await setupSettingsCache();Also applies to: 425-426
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte around lines 124 - 132, After calling navigateToTextChapterInDirection() in the doSwipe() function, you also need to refresh settingsCache to match the updated $refs state. The issue is that while swiping updates $refs, the page rendering uses settingsCache which becomes stale, causing text panels to display outdated cached references after swipe navigation. Update settingsCache following the navigation call in doSwipe() to ensure the rendered content stays synchronized with the current state. This same fix also applies to the code at lines 425-426.
♻️ Duplicate comments (1)
src/routes/text/+page.svelte (1)
798-798:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose the current reading panel to assistive technologies.
All three panel containers are hard-coded
aria-hidden="true". Since the current panel rotates among indices, the visible chapter is hidden from screen readers. Makearia-hiddenconditional on the panel’s logical position, or move it to the panel wrapper with the same condition.Suggested fix
- aria-hidden="true" + aria-hidden={panels_X[0] !== 0}- aria-hidden="true" + aria-hidden={panels_X[1] !== 0}- aria-hidden="true" + aria-hidden={panels_X[2] !== 0}Also applies to: 827-827, 857-857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte at line 798, The three panel containers in the file each have aria-hidden="true" hard-coded, which hides the currently visible panel from screen readers. Make the aria-hidden attribute conditional based on the panel's visibility state or logical position so that only the non-visible panels are hidden from assistive technologies. Update all three panel container instances to use a conditional expression for the aria-hidden attribute that evaluates to false when the panel is the current/active panel and true when it is not.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/components/ChapterSelector.svelte`:
- Line 182: Optional chaining in the tabColor variable assignment only protects
the initial $s reference but leaves nested property accesses unguarded. When $s
exists but ui.selector.tabs is missing, accessing ['color'] directly will throw
an error. In ChapterSelector.svelte at line 182, modify the statement to apply
optional chaining to both nested property accesses by changing
$s?.['ui.selector.tabs']['color'] to $s?.['ui.selector.tabs']?.['color']. Apply
the identical fix to BookSelector.svelte at line 331 where the same pattern
exists.
In `@src/routes/text/`+page.svelte:
- Around line 270-275: The ResizeObserver callback in the measure function only
updates panels_X for indexes 0 and 2, but does not update panels_X[1]. When the
width changes due to orientation or layout changes, if the current panel is at
index 1 (the center/visible position), it will be left out of the update and
move off center. Add an assignment to update panels_X[1] to maintain the center
position (typically set to 0) alongside the existing updates to panels_X[0] and
panels_X[2] in the ResizeObserver callback.
- Around line 248-267: The drag handler functions handleMouseDown and
handleMouseMove do not properly handle multi-touch gestures, allowing isDragging
to persist or respond to the wrong pointer when pinch or touch cancellation
occurs. Add pointerId tracking by storing event.pointerId in handleMouseDown and
validating it matches in handleMouseMove to ensure drag only responds to the
initiating pointer. Add setPointerCapture(event.pointerId) in handleMouseDown to
capture the pointer and prevent other handlers from interfering. Create a new
onpointercancel handler that resets isDragging to false and releases the pointer
capture to clean up stale drag state when gestures are interrupted.
---
Outside diff comments:
In `@src/routes/text/`+page.svelte:
- Around line 124-132: After calling navigateToTextChapterInDirection() in the
doSwipe() function, you also need to refresh settingsCache to match the updated
$refs state. The issue is that while swiping updates $refs, the page rendering
uses settingsCache which becomes stale, causing text panels to display outdated
cached references after swipe navigation. Update settingsCache following the
navigation call in doSwipe() to ensure the rendered content stays synchronized
with the current state. This same fix also applies to the code at lines 425-426.
---
Duplicate comments:
In `@src/routes/text/`+page.svelte:
- Line 798: The three panel containers in the file each have aria-hidden="true"
hard-coded, which hides the currently visible panel from screen readers. Make
the aria-hidden attribute conditional based on the panel's visibility state or
logical position so that only the non-visible panels are hidden from assistive
technologies. Update all three panel container instances to use a conditional
expression for the aria-hidden attribute that evaluates to false when the panel
is the current/active panel and true when it is not.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c354e33-0542-48ca-b694-5082b5040d69
📒 Files selected for processing (2)
src/lib/components/ChapterSelector.sveltesrc/routes/text/+page.svelte
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/text/+page.svelte (1)
124-132:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh
settingsCacheafter non-animated swipe navigation.The page now renders from
settingsCache, butdoSwipe()still only updates$refs. On story pages, swiping the illustration can navigate history/refs while the text panels keep rendering the previous cached references.Suggested fix
- await navigateToTextChapterInDirection(swipeDirection === 'right' ? -1 : 1); + await navigateToTextChapterInDirection(swipeDirection === 'right' ? -1 : 1); + await setupSettingsCache();Also applies to: 425-426
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte around lines 124 - 132, After calling navigateToTextChapterInDirection() in the doSwipe() function, you also need to refresh settingsCache to match the updated $refs state. The issue is that while swiping updates $refs, the page rendering uses settingsCache which becomes stale, causing text panels to display outdated cached references after swipe navigation. Update settingsCache following the navigation call in doSwipe() to ensure the rendered content stays synchronized with the current state. This same fix also applies to the code at lines 425-426.
♻️ Duplicate comments (1)
src/routes/text/+page.svelte (1)
798-798:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose the current reading panel to assistive technologies.
All three panel containers are hard-coded
aria-hidden="true". Since the current panel rotates among indices, the visible chapter is hidden from screen readers. Makearia-hiddenconditional on the panel’s logical position, or move it to the panel wrapper with the same condition.Suggested fix
- aria-hidden="true" + aria-hidden={panels_X[0] !== 0}- aria-hidden="true" + aria-hidden={panels_X[1] !== 0}- aria-hidden="true" + aria-hidden={panels_X[2] !== 0}Also applies to: 827-827, 857-857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte at line 798, The three panel containers in the file each have aria-hidden="true" hard-coded, which hides the currently visible panel from screen readers. Make the aria-hidden attribute conditional based on the panel's visibility state or logical position so that only the non-visible panels are hidden from assistive technologies. Update all three panel container instances to use a conditional expression for the aria-hidden attribute that evaluates to false when the panel is the current/active panel and true when it is not.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/components/ChapterSelector.svelte`:
- Line 182: Optional chaining in the tabColor variable assignment only protects
the initial $s reference but leaves nested property accesses unguarded. When $s
exists but ui.selector.tabs is missing, accessing ['color'] directly will throw
an error. In ChapterSelector.svelte at line 182, modify the statement to apply
optional chaining to both nested property accesses by changing
$s?.['ui.selector.tabs']['color'] to $s?.['ui.selector.tabs']?.['color']. Apply
the identical fix to BookSelector.svelte at line 331 where the same pattern
exists.
In `@src/routes/text/`+page.svelte:
- Around line 270-275: The ResizeObserver callback in the measure function only
updates panels_X for indexes 0 and 2, but does not update panels_X[1]. When the
width changes due to orientation or layout changes, if the current panel is at
index 1 (the center/visible position), it will be left out of the update and
move off center. Add an assignment to update panels_X[1] to maintain the center
position (typically set to 0) alongside the existing updates to panels_X[0] and
panels_X[2] in the ResizeObserver callback.
- Around line 248-267: The drag handler functions handleMouseDown and
handleMouseMove do not properly handle multi-touch gestures, allowing isDragging
to persist or respond to the wrong pointer when pinch or touch cancellation
occurs. Add pointerId tracking by storing event.pointerId in handleMouseDown and
validating it matches in handleMouseMove to ensure drag only responds to the
initiating pointer. Add setPointerCapture(event.pointerId) in handleMouseDown to
capture the pointer and prevent other handlers from interfering. Create a new
onpointercancel handler that resets isDragging to false and releases the pointer
capture to clean up stale drag state when gestures are interrupted.
---
Outside diff comments:
In `@src/routes/text/`+page.svelte:
- Around line 124-132: After calling navigateToTextChapterInDirection() in the
doSwipe() function, you also need to refresh settingsCache to match the updated
$refs state. The issue is that while swiping updates $refs, the page rendering
uses settingsCache which becomes stale, causing text panels to display outdated
cached references after swipe navigation. Update settingsCache following the
navigation call in doSwipe() to ensure the rendered content stays synchronized
with the current state. This same fix also applies to the code at lines 425-426.
---
Duplicate comments:
In `@src/routes/text/`+page.svelte:
- Line 798: The three panel containers in the file each have aria-hidden="true"
hard-coded, which hides the currently visible panel from screen readers. Make
the aria-hidden attribute conditional based on the panel's visibility state or
logical position so that only the non-visible panels are hidden from assistive
technologies. Update all three panel container instances to use a conditional
expression for the aria-hidden attribute that evaluates to false when the panel
is the current/active panel and true when it is not.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c354e33-0542-48ca-b694-5082b5040d69
📒 Files selected for processing (2)
src/lib/components/ChapterSelector.sveltesrc/routes/text/+page.svelte
🛑 Comments failed to post (3)
src/lib/components/ChapterSelector.svelte (1)
182-182:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Search for ui.selector.tabs definitions. # Expect: configs either define ui.selector.tabs.color everywhere, or code handles it being absent. rg -n -C2 'ui\.selector\.tabs|ui.selector.tabs' --glob '!node_modules/**'Repository: sillsdev/appbuilder-pwa
Length of output: 1414
🏁 Script executed:
#!/bin/bash # Check all config files to see if ui.selector.tabs is defined find . -type f -name "*.js" -o -name "*.json" -o -name "*.ts" | xargs grep -l "config" | head -20 | while read f; do if grep -q "ui.selector.tabs" "$f"; then echo "=== $f ===" grep -A3 "ui.selector.tabs" "$f" | head -8 fi doneRepository: sillsdev/appbuilder-pwa
Length of output: 104
🏁 Script executed:
#!/bin/bash # Check all instances where ['color'] is accessed on style config properties rg -n "\['ui\.selector\.tabs'\]\['color'\]|\['ui.selector.tabs'\]\['color'\]" --glob '!node_modules/**'Repository: sillsdev/appbuilder-pwa
Length of output: 284
🏁 Script executed:
#!/bin/bash # Verify TabsMenu.svelte line 46 to see complete context sed -n '40,50p' src/lib/components/TabsMenu.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 498
🏁 Script executed:
#!/bin/bash # Check BookSelector.svelte to see if the same issue exists there and context sed -n '325,335p' src/lib/components/BookSelector.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 396
🏁 Script executed:
#!/bin/bash # Check if there are any runtime guards or fallbacks for ui.selector.tabs elsewhere rg -n "ui.selector.tabs" --glob '!node_modules/**' -B3 -A3Repository: sillsdev/appbuilder-pwa
Length of output: 2130
🏁 Script executed:
#!/bin/bash # Check the type definitions to see if ui.selector.tabs is required or optional fd -t f "\.(ts|d\.ts)$" | xargs grep -l "ui.selector.tabs" 2>/dev/null | head -5Repository: sillsdev/appbuilder-pwa
Length of output: 49
Add optional chaining to protect access to
colorproperty.
$s?.['ui.selector.tabs']['color']still throws when$sexists butui.selector.tabsis missing from the config. The optional chaining on$sonly guards that reference; the nested property access needs its own guard.This same issue exists in both
ChapterSelector.svelte:182andBookSelector.svelte:331.Suggested fix
- let tabColor = $derived($s?.['ui.selector.tabs']['color']); + let tabColor = $derived($s?.['ui.selector.tabs']?.['color']);Apply the same fix to
BookSelector.svelte:331.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let tabColor = $derived($s?.['ui.selector.tabs']?.['color']);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ChapterSelector.svelte` at line 182, Optional chaining in the tabColor variable assignment only protects the initial $s reference but leaves nested property accesses unguarded. When $s exists but ui.selector.tabs is missing, accessing ['color'] directly will throw an error. In ChapterSelector.svelte at line 182, modify the statement to apply optional chaining to both nested property accesses by changing $s?.['ui.selector.tabs']['color'] to $s?.['ui.selector.tabs']?.['color']. Apply the identical fix to BookSelector.svelte at line 331 where the same pattern exists.Source: Learnings
src/routes/text/+page.svelte (2)
248-267:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect whether drag navigation tracks pointer identity/cancel events. # Expect after the fix: active pointerId/isPrimary handling and onpointercancel cleanup. sed -n '248,268p;635,640p' src/routes/text/+page.svelte rg -n -C2 '\bpointerId\b|\bonpointercancel\b|\bsetPointerCapture\b|\bisPrimary\b' src/routes/text/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 1055
🏁 Script executed:
# Find the handleMouseUp implementation and check for other pointer/gesture handlers rg -n 'function handleMouseUp|handleMouseUp\s*=' src/routes/text/+page.svelte -A 5 # Check for pinch, touch, or gesture handling rg -n -i 'pinch|gesture|ontouchstart|ontouchend|wheel' src/routes/text/+page.svelte # Check all pointer-related event bindings rg -n 'on(pointer|touch|mouse|wheel)' src/routes/text/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 1360
🏁 Script executed:
# Check if the elements with onpointerdown are the same as those with pinch sed -n '795,805p;824,834p;855,865p' src/routes/text/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 1661
Add pointer tracking and cancel handler to prevent stale drag state with multi-touch gestures.
The reading node divs support pinch gestures but use the same
onpointerdownfor drag navigation without tracking pointer identity or handling cancellation. When a pinch or touch cancellation occurs,isDraggingcan remaintrueor the drag can operate on the wrong pointer, causing unintended chapter navigation.Add:
pointerIdtracking inhandleMouseDownand validation inhandleMouseMoveonpointercancelhandler to clean upisDraggingand any active state- Pointer capture via
setPointerCaptureto ensure drag only responds to the initiating pointer🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte around lines 248 - 267, The drag handler functions handleMouseDown and handleMouseMove do not properly handle multi-touch gestures, allowing isDragging to persist or respond to the wrong pointer when pinch or touch cancellation occurs. Add pointerId tracking by storing event.pointerId in handleMouseDown and validating it matches in handleMouseMove to ensure drag only responds to the initiating pointer. Add setPointerCapture(event.pointerId) in handleMouseDown to capture the pointer and prevent other handlers from interfering. Create a new onpointercancel handler that resets isDragging to false and releases the pointer capture to clean up stale drag state when gestures are interrupted.
270-275:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve rotated panel positions when the width changes.
After a swipe, the current panel can be any index, e.g.
[draggableWidth, -draggableWidth, 0]. TheResizeObserverhard-resets only indexes 0 and 2, which moves the current panel off center after orientation or layout changes.Suggested fix
const observer = new ResizeObserver(([entry]) => { - draggableWidth = entry.contentRect.width; - panels_X[0] = -draggableWidth; - panels_X[2] = draggableWidth; + const nextWidth = entry.contentRect.width; + const uninitialized = panels_X.every((position) => position === 0); + + draggableWidth = nextWidth; + panels_X = uninitialized + ? [-nextWidth, 0, nextWidth] + : panels_X.map((position) => + position === 0 ? 0 : Math.sign(position) * nextWidth + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/text/`+page.svelte around lines 270 - 275, The ResizeObserver callback in the measure function only updates panels_X for indexes 0 and 2, but does not update panels_X[1]. When the width changes due to orientation or layout changes, if the current panel is at index 1 (the center/visible position), it will be left out of the update and move off center. Add an assignment to update panels_X[1] to maintain the center position (typically set to 0) alongside the existing updates to panels_X[0] and panels_X[2] in the ResizeObserver callback.
I wasn't planning on doing the animation because the selector could navigate to chapters in any direction and distance and thought it would be okay to just switch out the chapters. I've been using myk.global.bible as a reference as mentioned in earlier comments, and it doesn't use an animation for the chapter selector |
Understood. You probably shouldn't have any changes in ChapterSelector then?? |
Since I used three panels, not directly reliant on |
What data are you getting from the chapter selector that would not be accessible from the |
I am just detecting when the action of selecting a chapter is complete. I tried using an $effect to see changes to I just access the chapter selector and trigger the custom event when a new chapter is selected. Do you think there would be a better way for me to do this? |
That's actually a valid point. I wasn't thinking about the text loading. You could probably do something more along what I was asking for, but what you have now is probably good then. I guess just document it in a comment? |
3 participants
current chapter
(Adjustments can be made to
minSlideDistance)Summary by CodeRabbit
New Features
Refactor
Bug Fixes / UI