Upgrade remaining components to Svelte 5 syntax#962
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigration to Svelte 5 runes across the codebase: replace legacy props/reactivity/event directives with ChangesSvelte 5 Migration: Runes, Props API, and Asset/Navigation Helpers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/FontSelector.svelte`:
- Around line 13-15: The modal opens with a stale selectedFont value; update
selectedFont from the currentFont store/value before showing the dialog. In the
export function showModal() (and the equivalent open/submit path around the same
modal usage), read the latest currentFont (or $currentFont) and assign it to
selectedFont immediately prior to calling modal.showModal(), ensuring the
selection reflects the currentFont when the modal opens.
In `@src/lib/components/HistoryCard.svelte`:
- Line 55: The inline style on the clickable div using "text-decorationnone;" is
invalid and being ignored; update the div with the correct CSS declaration
(e.g., "text-decoration: none;") or remove the style attribute entirely if not
needed, by editing the element that contains onclick={onHistoryClick} to use a
valid style string or class name so the intended no-text-decoration behavior is
applied.
In `@src/lib/components/StackView.svelte`:
- Around line 118-121: In StackView.svelte change the onclick handler so
insideClick receives the actual clicked element instead of the outer container:
keep e.stopPropagation(), then compute const clicked = (e.target as
HTMLElement).closest('a') ?? e.target and call insideClick(clicked); this
ensures insideClick (used to check data-start-ref or class ref-link) sees the
generated <a> even when wrapped by tooltips or other wrappers.
In `@src/routes/plans/`[id]/settings/+page.svelte:
- Around line 21-24: Compute a single guarded planId from data.planConfig (e.g.
const planId = data.planConfig?.id) and use that in backNavigation and the start
action instead of accessing data.planConfig directly; if planId is missing do an
early return (or show a fallback) to avoid routing to `/plans/undefined` or
throwing on click—update the backNavigation function (which calls
gotoRoute(`/plans/${id}`)) and the other handler around lines 52-54 to use this
guarded planId and return immediately when it’s falsy.
🪄 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: 27d0830f-4f46-4c56-9852-9c0442931502
📒 Files selected for processing (93)
eslint.config.jssrc/lib/components/AudioPlaybackSpeed.sveltesrc/lib/components/ContentCarousel.sveltesrc/lib/components/ContentGrid.sveltesrc/lib/components/ContentHeading.sveltesrc/lib/components/ContentSingle.sveltesrc/lib/components/FontList.sveltesrc/lib/components/FontSelector.sveltesrc/lib/components/HistoryCard.sveltesrc/lib/components/HtmlBookView.sveltesrc/lib/components/IconCard.sveltesrc/lib/components/PlayButton.sveltesrc/lib/components/ScrolledContent.sveltesrc/lib/components/Sidebar.sveltesrc/lib/components/Slider.sveltesrc/lib/components/StackView.sveltesrc/lib/components/TextAppearanceSelector.sveltesrc/lib/data/plansData.tssrc/lib/icons/AboutIcon.sveltesrc/lib/icons/AccountIcon.sveltesrc/lib/icons/ArrowBackIcon.sveltesrc/lib/icons/ArrowForwardIcon.sveltesrc/lib/icons/BibleIcon.sveltesrc/lib/icons/BookmarkIcon.sveltesrc/lib/icons/BookmarkOutlineIcon.sveltesrc/lib/icons/CalendarMonthIcon.sveltesrc/lib/icons/CheckIcon.sveltesrc/lib/icons/CheckboxIcon.sveltesrc/lib/icons/CheckboxOutlineIcon.sveltesrc/lib/icons/ChevronIcon.sveltesrc/lib/icons/CopyContentIcon.sveltesrc/lib/icons/DeleteIcon.sveltesrc/lib/icons/DeleteSweepIcon.sveltesrc/lib/icons/DropdownIcon.sveltesrc/lib/icons/EditIcon.sveltesrc/lib/icons/HamburgerIcon.sveltesrc/lib/icons/HighlightIcon.sveltesrc/lib/icons/HistoryIcon.sveltesrc/lib/icons/HomeIcon.sveltesrc/lib/icons/InfoIcon.sveltesrc/lib/icons/LaunchIcon.sveltesrc/lib/icons/LineSpacingIcon.sveltesrc/lib/icons/MoreVertIcon.sveltesrc/lib/icons/NoteIcon.sveltesrc/lib/icons/SearchIcon.sveltesrc/lib/icons/SettingsIcon.sveltesrc/lib/icons/ShareIcon.sveltesrc/lib/icons/SideBySideIcon.sveltesrc/lib/icons/SinglePaneIcon.sveltesrc/lib/icons/SortIcon.sveltesrc/lib/icons/TextAppearanceIcon.sveltesrc/lib/icons/TriangleLeftIcon.sveltesrc/lib/icons/TriangleRightIcon.sveltesrc/lib/icons/VerseByVerseIcon.sveltesrc/lib/icons/audio/PauseIcon.sveltesrc/lib/icons/audio/PauseOutlineCircleIcon.sveltesrc/lib/icons/audio/PlayFillCircleIcon.sveltesrc/lib/icons/audio/PlayIcon.sveltesrc/lib/icons/audio/PlayOutlineCircleIcon.sveltesrc/lib/icons/audio/PlayRepeatIcon.sveltesrc/lib/icons/audio/RepeatIcon.sveltesrc/lib/icons/image/BlurIcon.sveltesrc/lib/icons/image/BrightnessIcon.sveltesrc/lib/icons/image/ContrastIcon.sveltesrc/lib/icons/image/FontChoiceIcon.sveltesrc/lib/icons/image/FormatAlignCenterIcon.sveltesrc/lib/icons/image/FormatAlignLeftIcon.sveltesrc/lib/icons/image/FormatAlignRightIcon.sveltesrc/lib/icons/image/FormatBoldIcon.sveltesrc/lib/icons/image/FormatColorFillIcon.sveltesrc/lib/icons/image/FormatItalicIcon.sveltesrc/lib/icons/image/FormatLineSpacingIcon.sveltesrc/lib/icons/image/ImageSingleIcon.sveltesrc/lib/icons/image/ImagesIcon.sveltesrc/lib/icons/image/LetterSpacingIcon.sveltesrc/lib/icons/image/OpacityIcon.sveltesrc/lib/icons/image/ReferenceIcon.sveltesrc/lib/icons/image/ResizeArrowsIcon.sveltesrc/lib/icons/image/SaturationIcon.sveltesrc/lib/icons/image/TextGlowIcon.sveltesrc/lib/icons/image/TextShadowIcon.sveltesrc/lib/icons/image/TextShadowNoneIcon.sveltesrc/lib/icons/image/TextWidthIcon.sveltesrc/lib/navigate/index.tssrc/lib/video/index.tssrc/routes/+page.sveltesrc/routes/bookmarks/+page.sveltesrc/routes/contents/[id]/+page.sveltesrc/routes/dev/icons/+page.sveltesrc/routes/lexicon/+page.sveltesrc/routes/notes/+page.sveltesrc/routes/plans/[id]/settings/+page.sveltesrc/routes/text/+page.svelte
💤 Files with no reviewable changes (3)
- src/lib/data/plansData.ts
- src/lib/components/ScrolledContent.svelte
- src/routes/lexicon/+page.svelte
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/lib/components/HistoryCard.svelte (1)
55-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a style declaration (or real CSS class), not a CSS rule string in
class.Line 55 uses
class="text-decoration:none;", which won’t apply the intended styling.Suggested fix
- <div class="text-decoration:none;" onclick={onHistoryClick}> + <div style="text-decoration:none;" onclick={onHistoryClick}>🤖 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/HistoryCard.svelte` at line 55, The element uses class="text-decoration:none;" which is incorrect; replace that with a proper style or CSS class: either change the attribute to style="text-decoration: none;" on the div that uses onclick={onHistoryClick}, or create a CSS class (e.g. .no-underline) in HistoryCard.svelte's <style> block and set class="no-underline" on the div so the decoration is applied correctly.src/routes/plans/[id]/settings/+page.svelte (1)
22-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
planConfig?.idbefore navigation and start action.Line 24 can route to
/plans/undefinedwhendata.planConfig?.idis missing, and Line 54 can throw due todata.planConfig!.id. Reuse one guardedplanIdand early-return/fallback.Proposed fix
let { data }: Props = $props(); +const planId = $derived(data.planConfig?.id); async function startPlan(id: string) { await addPlanState({ id: id, state: 'started' }); goto(resolve(`/plans/${id}`)); } function backNavigation() { - const id = data.planConfig?.id; - goto(resolve(`/plans/${id}`)); + if (!planId) { + goto(resolve('/plans')); + return; + } + goto(resolve(`/plans/${planId}`)); } @@ <div class="plan-button" id="plan-continue" onclick={async function () { - await startPlan(data.planConfig!.id); + if (!planId) return; + await startPlan(planId); }} >Also applies to: 53-55
🤖 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/plans/`[id]/settings/+page.svelte around lines 22 - 25, Compute and reuse a guarded planId once (e.g., const planId = data.planConfig?.id) and early-return or show a fallback when it's undefined before performing navigation or actions; update backNavigation() to use planId and avoid calling goto(`/plans/${id}`) when planId is falsy, and replace all occurrences of data.planConfig!.id (e.g., the start action at the other site) with the guarded planId so no code attempts to navigate or start with an undefined id.
🤖 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.
Duplicate comments:
In `@src/lib/components/HistoryCard.svelte`:
- Line 55: The element uses class="text-decoration:none;" which is incorrect;
replace that with a proper style or CSS class: either change the attribute to
style="text-decoration: none;" on the div that uses onclick={onHistoryClick}, or
create a CSS class (e.g. .no-underline) in HistoryCard.svelte's <style> block
and set class="no-underline" on the div so the decoration is applied correctly.
In `@src/routes/plans/`[id]/settings/+page.svelte:
- Around line 22-25: Compute and reuse a guarded planId once (e.g., const planId
= data.planConfig?.id) and early-return or show a fallback when it's undefined
before performing navigation or actions; update backNavigation() to use planId
and avoid calling goto(`/plans/${id}`) when planId is falsy, and replace all
occurrences of data.planConfig!.id (e.g., the start action at the other site)
with the guarded planId so no code attempts to navigate or start with an
undefined id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32912d66-6913-4f41-a3bf-b67ce26065a6
📒 Files selected for processing (24)
src/lib/components/BookSelector.sveltesrc/lib/components/BottomNavigationBar.sveltesrc/lib/components/ChapterSelector.sveltesrc/lib/components/FontSelector.sveltesrc/lib/components/HistoryCard.sveltesrc/lib/components/Navbar.sveltesrc/lib/components/NoteDialog.sveltesrc/lib/components/PlanStopDialog.sveltesrc/lib/components/ScriptureViewSofria.sveltesrc/lib/components/Sidebar.sveltesrc/lib/components/TextSelectionToolbar.sveltesrc/lib/navigate/index.tssrc/routes/+page.sveltesrc/routes/bookmarks/+page.sveltesrc/routes/contents/[id]/+page.sveltesrc/routes/highlights/+page.sveltesrc/routes/lexicon/+layout.sveltesrc/routes/lexicon/+page.sveltesrc/routes/notes/+page.sveltesrc/routes/plans/+page.sveltesrc/routes/plans/[id]/+page.sveltesrc/routes/plans/[id]/settings/+page.sveltesrc/routes/search/[collection]/[[savedResults]]/+page.sveltesrc/routes/text/+page.svelte
eomerdws
left a comment
There was a problem hiding this comment.
Awesome job.. It honestly took me a bit to realize that both goto and resolve where a svelte way of doing code. I don't think my questions are show stoppers so if you don't mind answering them for history sake otherwise I think this is an easy approve.
Closes #819, #890
Summary by CodeRabbit
Refactor
Chores