Button loading uses Spinner #1843
Conversation
|
@jjagielka is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReplaces the inline SVG spinner in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Button
participant Spinner
participant Theme
User->>Button: render (color="light", loading=true)
activate Button
Button->>Spinner: render(type="loading", color=actualColor, size="4")
deactivate Button
activate Spinner
Spinner->>Theme: resolve color variant for "light"
activate Theme
Theme-->>Spinner: classes (fill-white text-gray-900)
deactivate Theme
Spinner-->>User: render loading SVG with resolved classes
deactivate Spinner
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/buttons/Button.svelte(2 hunks)src/lib/spinner/Spinner.svelte(1 hunks)src/lib/spinner/theme.ts(2 hunks)src/lib/types.ts(2 hunks)src/routes/docs-examples/components/spinner/Type.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/lib/types.ts
🧬 Code graph analysis (1)
src/lib/types.ts (1)
src/lib/spinner/theme.ts (1)
SpinnerVaraiants(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
🔇 Additional comments (6)
src/routes/docs-examples/components/spinner/Type.svelte (1)
10-10: LGTM! Documentation example added.The new example demonstrates the loading spinner type with the light color variant, aligning with the new Spinner API.
src/lib/spinner/theme.ts (2)
14-14: LGTM! New spinner variants added correctly.The new
loadingtype withanimate-spinanimation and the three new color variants (light,dark,alternative) properly support the Button component's loading state integration.Also applies to: 37-39
13-13: Theorbitvariant empty string is intentional and correct.The orbit spinner uses SVG's native
<animateTransform>element to rotate three circles continuously, which doesn't require Tailwind CSS animation classes. Unlike other types (default,pulse,loading) that rely on CSS animations, the orbit type's animation is handled entirely by SVG animation primitives. The empty string is the appropriate value for this variant.src/lib/types.ts (2)
1554-1557: LGTM! SpinnerProps correctly refactored to use variants.The change properly delegates
colorandtypehandling toSpinnerVaraiantswhile preservingcurrentFillandcurrentColoras explicit fields. This aligns with the new variant system introduced in the spinner theme.
981-981: Minor whitespace change, no semantic impact.The formatting adjustment in the
onselectcallback type has no functional impact.src/lib/buttons/Button.svelte (1)
7-7: No color type compatibility issue exists.All values of
ButtonProps["color"]are valid forSpinnerProps["color"]. The Button component uses only thebuttontheme variant (which has 23 supported colors), and all of these colors are supported by the Spinner component. The gradient-only color variants (e.g.,purpleToBlue,cyanToBlue) belong toGradientButtonProps, notButtonProps, and are never passed to the Spinner, which only receives the button component's colors.Likely an incorrect or invalid review comment.
|
Oops! Haven't seen this is already tackled by PR #1838 |
1 participant
🔗 Related issue (optional)
Closes #1839
📑 Description
Button in loading mode used hard coded
svg. Not it is changed toSpinnerthat follows the button color.🔍 PR Type
🚦 PR Status
✅ Checklist
pnpm check && pnpm test:e2eand all tests passapi-checkdirectory if a component API changedmainbranch (not the published npm version)🧪 Screenshots / Demos (optional)
ℹ️ Additional Information
Summary by CodeRabbit
New Features
Documentation