-
-
Notifications
You must be signed in to change notification settings - Fork 10
breaking: Revert svelte 5 ui lib to Flowbite Svelte (#2054) #3061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… and add flowbite-svelte (#2054)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/forgot_password/+page.svelte (1)
91-91: Changecolor="primary"to a valid Flowbite Svelte Button color.The
color="primary"is not a valid value for the Button component. According to Flowbite Svelte documentation, valid color options are:alternative,dark,light,blue,green,red,yellow, orpurple.src/lib/components/SubmissionStatus/UpdatingDropdown.svelte (2)
78-88: Fragile DOM-based form submission withsetTimeout.Using
setTimeoutwith a 10ms delay to query the DOM for the submit button is a race condition risk. The form might not be rendered in time, especially on slower devices or when the component tree is complex.Consider using a more reliable approach:
Suggested improvement using tick()
+import { tick } from 'svelte'; + - function handleClick(submissionStatus: { + async function handleClick(submissionStatus: { innerId: string; innerName: string; labelName: string; - }): void { + }): Promise<void> { selectedSubmissionStatus = submissionStatus; showForm = true; - // Submit after the form is rendered. - setTimeout(() => { - const submitButton = document.querySelector( - '#submissionStatusForm button[type="submit"]', - ) as HTMLButtonElement; - - if (submitButton) { - // Submit the form via the enhance directive by clicking the button. - submitButton.click(); - } - }, 10); + // Wait for Svelte to update the DOM + await tick(); + const submitButton = document.querySelector( + '#submissionStatusForm button[type="submit"]', + ) as HTMLButtonElement; + + if (submitButton) { + submitButton.click(); + } }
119-137: Missingresponse.okcheck before parsing JSON.Calling
response.json()without first checkingresponse.okcan lead to unexpected behavior when the server returns an error status (4xx/5xx) with a non-JSON body.Proposed fix
fetch(action, { method: 'POST', body: formData, headers: { Accept: 'application/json', }, }) - .then((response) => response.json()) - .then(() => { + .then((response) => { + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + return response.json(); + }) + .then(() => { const updatedTaskResult = updateTaskResult(taskResult, submissionStatus); onupdate(updatedTaskResult); })
🤖 Fix all issues with AI agents
In
`@docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/investigation.md`:
- Line 95: Remove the extra spaces inside the emphasis markers in the heading
text "space-x-_ / space-y-_ セレクタ変更" (and the other occurrence noted at the same
section around the second instance) so that MD037 no longer flags spaces within
the emphasis; edit the markdown to use contiguous emphasis markers (e.g., change
"* space-x-_*" style to "*space-x-*") ensuring both the first occurrence and the
one around line 132 follow the same spacing fix.
In
`@docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md`:
- Line 70: The markdown line contains a bare URL "http://localhost:5174/" which
triggers MD034; update the text in the docs by wrapping the URL in angle
brackets like <http://localhost:5174/> or converting it to a markdown link
(e.g., [http://localhost:5174/](http://localhost:5174/)) so the URL is not bare
and markdownlint MD034 is satisfied.
- Line 451: Edit the Markdown line containing "space-y-_ / space-x-_" to remove
spaces inside the emphasis markers so the underscores are not separated by
spaces (e.g., make it "space-y-_/space-x-_" or wrap the terms in backticks to
render them as literal code); update the occurrence referenced in the file to
use the corrected form for "space-y-_ / space-x-_" so MD037 no longer flags
emphasis spacing.
- Around line 36-47: The fenced code block containing the Japanese bullet list
("前回の失敗:... ダークモード toggle が消失") needs a language identifier to satisfy
markdownlint MD040—change the opening fence from ``` to ```text (keeping the
same closing fence ```), so the block becomes a text-coded fence and linting
passes; ensure the opening/closing backtick counts match and no other content is
altered.
In `@lefthook.yml`:
- Around line 14-16: The lefthook hook currently runs the eslint job in parallel
with other jobs (including the formatter), which can cause race conditions where
ESLint lints unformatted files; update the lefthook.yml to run the formatter
(prettier or the `prettier-check` job) before the `eslint` job or disable
parallel execution for the hook so `name: eslint` executes after the formatter
completes, ensuring files are formatted prior to linting and avoiding spurious
ESLint errors.
- Around line 10-12: The prettier-check job can race with the format job when
parallel: true; update lefthook.yml to either remove the "prettier-check" job
entirely (since "prettier --write" already formats) or ensure ordering by
disabling parallel execution and running "format" before "prettier-check" (e.g.,
set parallel: false or move "prettier-check" into a sequential phase/piped job
that depends on "format"); refer to the job name "prettier-check" and the
formatting job (e.g., "format" / the job that runs "prettier --write") when
making the change.
In `@src/lib/components/LabelWithTooltips.svelte`:
- Around line 20-24: The dynamic Tailwind class `max-w-[${tooltipWidth}px]` in
LabelWithTooltips.svelte won't be generated by Tailwind JIT; change the Tooltip
usage to apply the width via an inline style (e.g., set style with `max-width:
${tooltipWidth}px`) and keep the existing TOOLTIP_CLASS_BASE in the class
attribute, or if tooltipWidth is one of a finite set, map it to predefined
Tailwind classes instead; update the Tooltip element (identified as Tooltip,
tooltipWidth, and TOOLTIP_CLASS_BASE) accordingly.
In `@src/lib/components/TaskGrades/GradeGuidelineTable.svelte`:
- Around line 28-30: The container div in GradeGuidelineTable.svelte uses a
bright dark-mode border class `dark:border-gray-100` that clashes with the table
body’s `dark:divide-gray-700`; update the container’s dark border class to
`dark:border-gray-700` (i.e., change the class on the outer container div
element) so the outer border matches the TableBody row dividers and maintains
consistent dark-mode styling.
In `@src/lib/components/TaskListSorted.svelte`:
- Line 25: The border color in TaskListSorted.svelte uses an inverted dark-mode
class ("dark:border-gray-100") causing an overly light border in dark mode;
change the dark-mode utility to a darker gray (for example
"dark:border-gray-700" or "dark:border-gray-600") on the container with class
"rounded-md border border-gray-200 dark:border-gray-100 overflow-hidden" so the
border in dark mode matches typical darker shades; update that class string
accordingly.
In `@src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte`:
- Around line 163-165: The header cell in WorkBookTasksTable.svelte currently
uses a visually-hidden label "編集" but the column contains the delete action;
update the TableHeadCell's sr-only span to the correct label for deletion (e.g.,
"削除" or the appropriate i18n key) so screen readers announce the delete column
correctly, and ensure any corresponding delete button aria-labels remain
consistent with this header.
- Around line 211-217: The contenteditable <td> in WorkBookTasksTable.svelte
needs an explicit accessible role and label so screen readers treat it as a
textbox; update the element that uses contenteditable and oninput={(event) =>
updateComment(index, event)} (and handlers handleFocus/handleBlur) to include
role="textbox" and a descriptive aria-label or aria-labelledby (e.g., using the
task title or index to build a unique label), and add aria-multiline="true" if
comments can span lines; keep existing event handlers and class directives
intact.
In `@src/routes/workbooks/`[slug]/+page.svelte:
- Line 152: The border class in the workbook page wrapper uses an inconsistent
dark-mode token; update the div with class "overflow-auto rounded-md border
border-gray-200 dark:border-gray-100" to use the same dark-mode border as the
rest of the PR by replacing dark:border-gray-100 with dark:border-gray-700
(locate the div in src/routes/workbooks/[slug]/+page.svelte, around the wrapper
that has "overflow-auto rounded-md border ...").
In `@tests/custom-colors.spec.ts`:
- Line 6: Rename the test suite description string in the test.describe call so
it correctly references Tailwind CSS v4 instead of v3; locate the
test.describe('TailwindCSS v3 configuration', ...) line in
tests/custom-colors.spec.ts and change the literal to something like
'TailwindCSS v4 configuration' (or another accurate v4 phrasing) to match the PR
migration.
- Around line 56-74: The test named "xs breakpoint is available (or custom
breakpoints)" only checks totalCssSize; update it to actually validate xs
breakpoint classes by reading the concatenated CSS from cssFiles (use cssFiles,
cssDir and totalCssSize variables) and assert that the CSS contains either the
literal "xs:" class prefix or relevant media query tokens like "@media
(min-width" (or, if you prefer the simpler fix, rename the test string to "CSS
output is generated with sufficient content" to reflect the current assertion
rather than adding breakpoint-specific checks).
🧹 Nitpick comments (14)
src/lib/components/TaskListSorted.svelte (2)
27-27: Consider adding dark mode background for TableHead.
bg-gray-100is applied without a dark mode variant. This may appear inconsistent in dark mode if the table body has a different background.Suggested improvement
- <TableHead class="text-md bg-gray-100"> + <TableHead class="text-md bg-gray-100 dark:bg-gray-800">
23-24: Outstanding TODO/FIXME items flagged.There are pending items for responsive design and icon size alignment. Would you like me to help address these issues or open tracking issues for them?
src/lib/components/TaskSearchBox.svelte (1)
173-179: Consider clarifying the comment.The custom
<li>element implementation with conditional highlighting is a reasonable workaround. However, the comment on line 173 could be clearer.📝 Suggested comment improvement
- <!-- WHY: Use a custom <li> element with conditional classes for highlighting, due not to be able to find attribute. --> + <!-- WHY: Use a custom <li> element with conditional classes for highlighting, as Flowbite Svelte's ListgroupItem does not expose the necessary attribute for focus styling. -->playwright.config.ts (1)
9-11: Consider skippingwebServerwhen running against a remoteBASE_URL.
IfBASE_URLpoints to a deployed site, the local build+preview can be redundant and may conflict on port 4173.♻️ Optional gating via env flag
-const config: PlaywrightTestConfig = { - webServer: { - command: 'pnpm build && pnpm preview', - port: 4173, - timeout: 10000 * 1000, - }, - use: { - baseURL: process.env.BASE_URL ?? 'http://localhost:4173', - }, +const skipWebServer = process.env.SKIP_WEB_SERVER === '1'; +const config: PlaywrightTestConfig = { + webServer: skipWebServer + ? undefined + : { + command: 'pnpm build && pnpm preview', + port: 4173, + timeout: 10000 * 1000, + }, + use: { + baseURL: process.env.BASE_URL ?? 'http://localhost:4173', + },package.json (1)
34-61: Check whetherflowbiteandflowbite-svelteshould be runtime dependencies.
If production installs prune devDependencies before build (e.g.,pnpm install --prod), the Svelte build will fail. Consider moving these todependenciesif your deploy path does not install devDependencies.♻️ Suggested move to dependencies (if needed)
"devDependencies": { - "flowbite": "3.1.2", - "flowbite-svelte": "1.31.0", ... }, "dependencies": { + "flowbite": "3.1.2", + "flowbite-svelte": "1.31.0", ... }src/lib/components/WorkBooks/WorkBookList.svelte (1)
43-48: Consider synchronizing local state with the store.The local
showReplenishmentWorkbooksstate is initialized fromreplenishmentWorkBooksStore.canView()but won't update if the store changes externally (e.g., from another component or persistence layer). If external updates are possible, consider adding an$effectto keep them in sync:♻️ Optional: Add effect to sync with store
let showReplenishmentWorkbooks = $state(replenishmentWorkBooksStore.canView()); +$effect(() => { + showReplenishmentWorkbooks = replenishmentWorkBooksStore.canView(); +}); + function handleReplenishmentWorkbooks() { showReplenishmentWorkbooks = !showReplenishmentWorkbooks; replenishmentWorkBooksStore.toggleView(); }tests/navbar.spec.ts (2)
58-64: Consider more robust selectors for mobile menu elements.The hamburger button selector
nav button:not([aria-label])is fragile and may break if the Flowbite component changes its structure or if other buttons without aria-labels are added. Similarly,div[role="none"] uldepends on specific DOM implementation.Consider using data-testid attributes or more specific Flowbite-provided selectors when the mobile navbar issue is fixed.
31-39: Bounding box null handling could be more consistent.The null handling for
bboxis inconsistent - using optional chaining (bbox?.width) for some properties but fallback (bbox?.x || 0) for others.Consider consistent null handling
for (let i = 0; i < Math.min(count, 3); i++) { const item = navItems.nth(i); const bbox = await item.boundingBox(); + expect(bbox).not.toBeNull(); - expect(bbox?.width).toBeGreaterThan(0); - expect(bbox?.height).toBeGreaterThan(0); + expect(bbox!.width).toBeGreaterThan(0); + expect(bbox!.height).toBeGreaterThan(0); // Verify items fit within navbar viewport - expect(bbox?.x || 0).toBeGreaterThanOrEqual(0); + expect(bbox!.x).toBeGreaterThanOrEqual(0); }tests/dark-mode.spec.ts (2)
34-39: Inconsistent fixture usage across tests.The tests "dark toggle button is visible" (line 34) and "dark mode toggle switches theme" (line 62) use the default
pagefixture while other tests use device-specific fixtures (iPhonePage,desktopPage). This inconsistency may lead to different viewport behaviors. Consider using explicit device fixtures for all tests or documenting why defaultpageis acceptable for these cases.Also applies to: 62-75
72-74: Class comparison may pass incorrectly if both values are null.If the
htmlelement has no class attribute, bothinitialClassandafterClasswould benull, andnull !== nullis false, so the assertion would fail correctly. However, consider using a more explicit assertion to verify thedarkclass was actually toggled.More explicit dark mode assertion
// Verify class changed (dark class toggled) const afterClass = await html.getAttribute('class'); - expect(initialClass).not.toBe(afterClass); + expect(afterClass).not.toBe(initialClass); + + // Verify dark class was actually toggled + const hasDarkInitially = initialClass?.includes('dark') ?? false; + const hasDarkAfter = afterClass?.includes('dark') ?? false; + expect(hasDarkInitially).not.toBe(hasDarkAfter);tests/custom-colors.spec.ts (1)
20-27: Try-catch fallback logic may mask errors and is potentially ineffective.The catch block catches all exceptions including permission errors, and the fallback does the same filtering operation that would have already succeeded or failed. The filter itself (
f.startsWith('0.') && f.endsWith('.css')) doesn't throw - onlyreaddirSynccould throw if the directory doesn't exist.Simplified approach
const cssDir = resolve('.svelte-kit/output/client/_app/immutable/assets'); - let cssFiles: string[] = []; - - try { - cssFiles = readdirSync(cssDir).filter( - (f: string) => f.startsWith('0.') && f.endsWith('.css'), - ); - } catch (e) { - // Fallback: search for all .css files - cssFiles = readdirSync(cssDir).filter((f: string) => f.endsWith('.css')); - } + const allCssFiles = readdirSync(cssDir).filter((f: string) => f.endsWith('.css')); + // Prefer 0.*.css files (main entry), fallback to any CSS file + let cssFiles = allCssFiles.filter((f: string) => f.startsWith('0.')); + if (cssFiles.length === 0) { + cssFiles = allCssFiles; + }docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/component-mapping.md (2)
90-90: Consider using proper headings instead of bold emphasis.Static analysis flagged several instances where bold text (
**text**) is used as a pseudo-heading. Using proper Markdown headings (###or####) would improve document structure and accessibility for screen readers.Lines affected: 90, 133, 159, 185, 351, 511, 779
Example fix for line 90
-**対応例:Navbar(Header.svelte の実装例)** +#### 対応例:Navbar(Header.svelte の実装例)
421-431: Minor inconsistency: documentation usesflex-shrink-0but implementation usesshrink-0.The example code in the documentation shows
flex-shrink-0(line 425), but the actual implementation inUpdatingDropdown.svelteusesshrink-0. While both work in Tailwind v4, updating the documentation to match the implementation would maintain consistency.Suggested fix
- class="flex-shrink-0 w-6 ml-auto cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-700 rounded p-1 transition" + class="shrink-0 w-6 ml-auto cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-700 rounded-sm p-1 transition"src/lib/components/InputFieldWrapper.svelte (1)
38-39: Consider simplifying the readonly conditional.The spread pattern works but can be simplified since
readonly={false}and omittingreadonlyare equivalent.Optional simplification
bind:value={inputValue} - {...isReadOnly ? { readonly: true } : {}} + readonly={isReadOnly} />
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/investigation.md
Show resolved
Hide resolved
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md
Outdated
Show resolved
Hide resolved
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md
Outdated
Show resolved
Hide resolved
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte`:
- Around line 151-152: The Table element in WorkBookTasksTable.svelte uses an
invalid Tailwind class `text-md`; replace it with a valid font-size utility
(e.g., `text-base` or `text-lg`) on the Table component (look for the Table
element with class "text-md table-fixed w-full") so Tailwind will apply the
intended font-size.
In `@tests/custom-colors.spec.ts`:
- Around line 20-27: The catch block re-invokes readdirSync(cssDir) which will
fail the same way; instead remove the redundant fallback and let the original
error propagate (or throw a new Error with clear context). Locate the try/catch
around readdirSync(cssDir) that assigns cssFiles (variable cssFiles) and either
rethrow the caught exception (throw e) or replace the catch body with: throw new
Error(`Failed to read cssDir "${cssDir}": ${e.message}`) so the failure is
surfaced rather than retrying the same directory.
♻️ Duplicate comments (3)
src/lib/components/LabelWithTooltips.svelte (1)
23-23: Dynamic CSS variable class still won't be generated by Tailwind.The
[--tooltip-width:${tooltipWidth}px]class is still dynamically interpolated at runtime. Tailwind's JIT compiler scans source files at build time and won't see resolved values like[--tooltip-width:280px], so no CSS will be generated for setting the variable.The
max-w-[var(--tooltip-width)]part is static and will work, but the variable needs to be set via inline style instead.Suggested fix
<Tooltip type="auto" triggeredBy="#{tooltipId}" - class={`[--tooltip-width:${tooltipWidth}px] max-w-[var(--tooltip-width)] ${TOOLTIP_CLASS_BASE}`} + class={`max-w-[var(--tooltip-width)] ${TOOLTIP_CLASS_BASE}`} + style={`--tooltip-width: ${tooltipWidth}px`} >src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte (1)
211-222: Previous review feedback not yet addressed: contenteditable cell needs ARIA attributes.The contenteditable
<td>should expose an explicit textbox role and label for screen readers, as noted in the prior review.🔧 Proposed fix (from prior review)
<td contenteditable="true" + role="textbox" + aria-label="一言(コメント・ヒント)" + aria-multiline="false" class="xs:text-lg hidden sm:table-cell text-gray-700 dark:text-gray-300 truncate" oninput={(event) => updateComment(index, event)} onfocus={handleFocus} onblur={handleBlur}docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md (1)
451-451: Fix MD037 emphasis spacing.The underscores in
space-y-_ / space-x-_may be interpreted as emphasis markers. Wrap them in backticks.Proposed fix
-- [ ] space-y-_ / space-x-_ による margin 変更を visual 確認 +- [ ] `space-y-*` / `space-x-*` による margin 変更を visual 確認
🧹 Nitpick comments (3)
src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte (1)
229-239: TheisDeletingguard is ineffective for synchronous operations.Since
confirm()is blocking andremoveWorkBookTask()is synchronous,isDeletingis set back tofalseimmediately in thefinallyblock before any subsequent click can occur. The button will never actually be disabled when the user could click it.Consider removing the state if it's not needed, or keep it if you plan to make deletion async later.
♻️ Option: Simplify by removing unused state
- let isDeleting = $state(false);<button type="button" class="flex justify-center items-center" onclick={() => { if (confirm('本当に削除しますか?')) { - try { - isDeleting = true; - removeWorkBookTask(task); - } finally { - isDeleting = false; - } + removeWorkBookTask(task); } }} - disabled={isDeleting} >src/app.css (1)
51-62: Empty CSS variable declarations may cause issues.These empty color variables (
--color-atcoder-gray: ;, etc.) will result in invalid CSS values if used. If these are intentional placeholders for future implementation, consider either:
- Removing them until values are defined
- Adding a comment explaining they're placeholders
- Setting fallback values
Option: Remove or comment the placeholders
- --color-atcoder-gray: ; - --color-atcoder-brown: ; - --color-atcoder-green: ; - --color-atcoder-cyan: ; - --color-atcoder-blue: ; - --color-atcoder-yellow: ; - --color-atcoder-orange: ; - --color-atcoder-red: ; + /* TODO: Define AtCoder rating colors when needed */ + /* --color-atcoder-gray: `#808080`; */ + /* --color-atcoder-brown: `#804000`; */ + /* ... */ --color-atcoder-bronze: `#e38a66`; - --color-atcoder-silver: ; - --color-atcoder-gold: ; + /* --color-atcoder-silver: `#c0c0c0`; */ + /* --color-atcoder-gold: `#ffd700`; */docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md (1)
133-185: Documentation example differs from actual test implementation.The example code uses
require('fs').readdirSyncand different assertions (e.g.,css.length > 10000for xs breakpoint) compared to the actualtests/custom-colors.spec.tswhich uses ES module imports and validates actual media query patterns. Consider updating this documentation to match the implemented tests for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/README-plan.md`:
- Line 283: Replace the bare URL
"https://github.com/themesberg/flowbite-svelte/issues/1710" with a proper
Markdown link to avoid MD034; update the line currently showing "- GitHub Issue:
https://github.com/themesberg/flowbite-svelte/issues/1710" to use descriptive
link text such as "GitHub Issue `#1710`" (e.g., "- GitHub Issue: [GitHub Issue
`#1710`](https://github.com/themesberg/flowbite-svelte/issues/1710)"), ensuring
the URL remains intact but is not presented bare.
- Line 291: The MD037 emphasis spacing warning comes from the checklist item
containing underscores ("space-y-_ / space-x-_"); fix it by preventing markdown
from treating underscores as emphasis—wrap the selectors in inline code (e.g.,
`space-y-_` / `space-x-_`) or escape the underscores (space-y-\_ / space-x-\_)
so the text renders literally; update the line that currently reads "space-y-_ /
space-x-_" accordingly.
In
`@docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md`:
- Around line 185-196: The test 'xs breakpoint media queries are generated in
CSS' references an undefined variable allCss; replace or assign it so the
assertion uses the actual CSS string: either change the expectation to use the
existing variable css (expect(css).toMatch(...)) or set allCss = css before the
expect line; update the assertion in that test accordingly to use the defined
variable css (or ensure allCss is assigned from css) and keep the regex check
for `@media`(min-width:26.25rem).
♻️ Duplicate comments (2)
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md (1)
463-463: MD037 already flagged for this line; please apply the earlier fix.src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte (1)
211-222: Add ARIA attributes for the contenteditable cell.The contenteditable cell still lacks
role="textbox"andaria-labelfor screen reader accessibility, as flagged in the previous review.
🧹 Nitpick comments (1)
src/lib/components/WorkBookTasks/WorkBookTasksTable.svelte (1)
224-244: TheisDeletingstate is ineffective for synchronous operations.
removeWorkBookTaskis synchronous, soisDeletingtoggles instantly within the same event loop tick—the button will never render in a disabled state. Either remove theisDeletinglogic or, if the intent is to prevent double-clicks during a future async operation, refactor accordingly.♻️ Simplified synchronous version
<button type="button" class="flex justify-center items-center" onclick={() => { if (confirm('本当に削除しますか?')) { - try { - isDeleting = true; - removeWorkBookTask(task); - } finally { - isDeleting = false; - } + removeWorkBookTask(task); } }} - disabled={isDeleting} >
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/README-plan.md
Outdated
Show resolved
Hide resolved
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/README-plan.md
Outdated
Show resolved
Hide resolved
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/testing-strategy.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/component-mapping.md`:
- Around line 179-198: Remove the short duplicate "カテゴリ3:外部ライブラリからの復帰(⭐⭐ 難易度中)"
block (the brief table and small Carousel example that imports
embla-carousel-svelte and flowbite-svelte) and keep the comprehensive Category 3
section (the detailed property mapping, full before/after examples, migration
notes and lessons learned). Move that comprehensive Category 3 so it sits
immediately after Category 2 and before Category 4 in the document, update the
surrounding headings/TOC to reflect the relocation, and delete the earlier brief
snippet to eliminate duplication.
🧹 Nitpick comments (2)
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/component-mapping.md (2)
34-177: Well-structured migration guidance for simpler components.The tables and code examples clearly demonstrate the migration path from svelte-5-ui-lib to Flowbite Svelte. The Navbar example at lines 92-120 effectively shows how state management simplifies with the new library.
Optional: Address markdownlint warnings
Static analysis flagged lines 90 and 133 for using bold emphasis instead of proper headings. Consider converting:
-**対応方法:** +### 対応方法-**対応例:Navbar(Header.svelte の実装例)** +#### 対応例:Navbar(Header.svelte の実装例)However, this is stylistic and the current format works well for subsection labels.
484-696: Comprehensive Modal, Toast, and other component migration patterns.The Modal section (4-2) includes an excellent comparison table showing the shift from
uiHelpers()to native<dialog>with Svelte 5 runes. The integration with SvelteKit Form Actions usinguse:enhanceis well-documented.The remaining components (Toast, Spinner, ButtonGroup, Footer) provide appropriate level of detail for their complexity.
Optional: Convert bold text to headings
For consistency with markdown best practices, consider converting bold subsection labels to proper headings:
Lines 351, 511:
-**Before (svelte-5-ui-lib)** +#### Before (svelte-5-ui-lib)
docs/dev-notes/2026-01-02/migrate-from-svelte-5-ui-lib-to-flowbite-svelte/component-mapping.md
Show resolved
Hide resolved
KATO-Hiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
close #2054
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.