- 
                Notifications
    
You must be signed in to change notification settings  - Fork 136
 
chore: refactoring and dependency updates #409
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
chore: refactoring and dependency updates #409
Conversation
…tiple components and remove unused utility
- Bump @types/node from ^20.17.30 to ^20.19.19 - Upgrade @vitejs/plugin-vue from 5.2.3 to 5.2.4 - Update @vitest/coverage-v8 from 3.1.1 to 3.2.4 - Upgrade @vue/eslint-config-typescript from 14.5.0 to 14.6.0 - Bump @vue/tsconfig from 0.7.0 to 0.8.1 - Update eslint from 9.24.0 to 9.36.0 - Upgrade eslint-plugin-import from ^2.31.0 to ^2.32.0 - Update eslint-plugin-unused-imports from ^4.1.4 to ^4.2.0 - Upgrade eslint-plugin-vue from 10.0.0 to 10.5.0 - Update eslint-plugin-vue-scoped-css from ^2.9.0 to ^2.12.0 - Bump neostandard from ^0.12.1 to ^0.12.2 - Upgrade tsc-alias from 1.8.15 to 1.8.16 - Update typescript from 5.8.3 to 5.9.3 - Upgrade vite from 6.3.1 to 6.3.6 - Update vitepress from 1.6.3 to 1.6.4 - Upgrade vitest from 3.1.1 to 3.2.4 - Update vue-tsc from 2.2.8 to 2.2.12 - Bump @tailwindcss/postcss from ^4.1.4 to ^4.1.14 - Upgrade @tailwindcss/vite from ^4.1.4 to ^4.1.14 - Update @vueuse/integrations from ^13.1.0 to ^13.9.0 - Upgrade nanoid from 5.1.5 to 5.1.6 - Update tailwind-merge from 3.2.0 to 3.3.1 - Upgrade tailwindcss from ^4.1.4 to ^4.1.14
- handle potential null values in getAccordionState and getFirstSlotVNode functions
…ses (lint errors)
…ackage.json - Bump @vitejs/plugin-vue from 5.2.4 to 6.0.1 - Upgrade jsdom from 26.1.0 to 27.0.0 - Update vite from 6.3.6 to 7.1.7 - Upgrade vue-tsc from 2.2.12 to 3.1.0 - Bump @vueuse/core from 12.8.2 to 13.9.0 - Change Node.js engine requirement from >=18.x to >=20.x
          ✅ Deploy Preview for sensational-seahorse-8635f8 ready!
 To edit notification comments on pull requests, go to your Netlify project configuration.  | 
    
| 
          
 Caution Review failedThe pull request is closed. WalkthroughThis PR removes the simplifyTailwindClasses utility, migrates components to useMergeClasses, adds null/undefined guards in several places (accordion state, toast pop, slot helper, autocomplete Enter handling), updates example templates/classes, and bumps Node and dependency versions. Changes
 Sequence Diagram(s)sequenceDiagram
  autonumber
  actor U as User
  participant A as FwbAutocomplete
  participant L as OptionsList
  U->>A: KeyDown Enter
  A->>L: retrieve highlighted option
  alt option defined
    A->>A: select(option)
    Note right of A: guarded select prevents calling with undefined
  else option undefined
    A-->>U: ignore Enter
  end
    sequenceDiagram
  autonumber
  participant C as Caller
  participant P as FwbToastProvider
  C->>P: popToast()
  P->>P: lastToast = toasts.at(-1)
  alt lastToast exists
    P->>P: lastId = lastToast.id
    P->>P: remove last toast
    P-->>C: lastId
  else no toasts
    P-->>C: ""
  end
  Note over P: added safety check for empty array to avoid undefined access
    Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
 Suggested labels
 Poem
 Pre-merge checks and finishing touches✅ Passed checks (3 passed)
 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/utils/FlowbiteThemable/FlowbiteThemableChild.vue (1)
4-4: Rename utilityuseMergeClassestomergeClasses
useMergeClassesis a plain function returning a string, not a Vue composable. Rename its definition insrc/composables/useMergeClasses.tsand update all imports (e.g. replaceuseMergeClasseswithmergeClasses) to align with naming conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
docs/components/card/examples/FwbCardExample.vue(1 hunks)docs/components/textarea/examples/FwbTextareaExampleDisabled.vue(2 hunks)docs/components/toastProvider/examples/UpdateToast.vue(1 hunks)package.json(1 hunks)src/components/FwbAccordion/composables/useAccordionState.ts(1 hunks)src/components/FwbAutocomplete/FwbAutocomplete.vue(4 hunks)src/components/FwbFileInput/composables/useFileInputClasses.ts(2 hunks)src/components/FwbRange/composables/useRangeClasses.ts(2 hunks)src/components/FwbTabs/composables/useTabClasses.ts(2 hunks)src/components/FwbTextarea/composables/useTextareaClasses.ts(1 hunks)src/components/FwbToast/FwbToastProvider.vue(1 hunks)src/components/utils/FlowbiteThemable/FlowbiteThemableChild.vue(2 hunks)src/utils/getFirstSlotNode.ts(1 hunks)src/utils/simplifyTailwindClasses.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/utils/simplifyTailwindClasses.ts
 
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/FwbRange/composables/useRangeClasses.ts (1)
src/composables/useMergeClasses.ts (1)
useMergeClasses(27-29)
src/components/FwbTextarea/composables/useTextareaClasses.ts (1)
src/composables/useMergeClasses.ts (1)
useMergeClasses(27-29)
src/components/FwbFileInput/composables/useFileInputClasses.ts (1)
src/composables/useMergeClasses.ts (1)
useMergeClasses(27-29)
src/components/FwbTabs/composables/useTabClasses.ts (1)
src/composables/useMergeClasses.ts (1)
useMergeClasses(27-29)
🔇 Additional comments (18)
docs/components/card/examples/FwbCardExample.vue (1)
3-6: LGTM! Clean formatting improvement.The multi-line formatting of the component attributes enhances readability and aligns with modern Vue formatting conventions.
src/components/FwbAccordion/composables/useAccordionState.ts (1)
38-39: LGTM! Safer null handling with explicit guard and nullish coalescing.The refactor correctly replaces
||with??and adds an explicit early return. The nullish coalescing operator is more semantically appropriate here sinceaccordionStates[accordionId]will only be an object or undefined, making??more precise than||.docs/components/textarea/examples/FwbTextareaExampleDisabled.vue (2)
3-3: LGTM! Grid layout improves example presentation.The addition of grid layout with gap-2 spacing provides better visual organization for the form elements.
25-28: LGTM! Button alignment complements grid layout.The
justify-self-startclass prevents the button from stretching full-width, which is appropriate for a submit button in this context.src/components/FwbAutocomplete/FwbAutocomplete.vue (5)
36-36: LGTM: Class reordering for consistency.The Tailwind utility classes have been reordered without changing functionality.
Also applies to: 41-41
88-88: Good improvement: Centers loading message horizontally.Adding
justify-centerimproves the visual alignment of the loading spinner and text.
90-90: LGTM: Consistent class ordering.The spinner classes match the reordering applied at line 36.
130-130: LGTM: Text class reordering.The validation and helper text classes have been reordered for consistency without changing the rendering.
Also applies to: 137-137
306-309: Good defensive guard: Prevents potential runtime error.The undefined check ensures
select()is only called with a valid option. This guards against edge cases wherefilteredOptionsmight change between keyboard navigation and selection (e.g., during async updates or race conditions).src/utils/getFirstSlotNode.ts (1)
19-20: Good defensive programming.The nullish coalescing operator ensures the return type strictly matches
VNode | nullrather than potentially returningundefined. While the length check on line 19 guarantees the element exists, this explicit guard improves type safety.docs/components/toastProvider/examples/UpdateToast.vue (1)
2-40: Documentation example updated appropriately.The template restructuring improves the example's layout and styling. The additional wrapper div and updated Tailwind classes align with modern best practices.
src/components/FwbToast/FwbToastProvider.vue (1)
50-51: Defensive guard improves robustness.The explicit check for
lastToastbefore accessing itsidproperty guards against potential undefined access. While the length check on line 46 already prevents empty array cases, this handles edge cases like concurrent modifications or sparse arrays.package.json (1)
48-66: Confirm npm scripts function correctly after major upgrades. Run the build, test, lint, and typecheck scripts locally to ensure ESLint 9.36.0, Vite 7.1.7, and TypeScript 5.9.3 haven’t introduced any regressions.src/components/FwbRange/composables/useRangeClasses.ts (1)
5-5: LGTM! Clean migration to useMergeClasses.The refactoring correctly replaces
simplifyTailwindClasseswithuseMergeClasses, maintaining the same class merging logic.Also applies to: 22-25
src/components/FwbTextarea/composables/useTextareaClasses.ts (1)
3-3: LGTM! Consistent refactoring with conditional logic preserved.The migration to
useMergeClassescorrectly maintains the conditional class application based on thecustomparameter.Also applies to: 11-14
src/components/FwbFileInput/composables/useFileInputClasses.ts (1)
3-3: LGTM! Straightforward migration with dynamic class construction.The refactoring correctly applies
useMergeClassesto merge the default classes with the dynamically constructed text size class.Also applies to: 12-15
src/components/FwbTabs/composables/useTabClasses.ts (2)
12-12: Good addition to the type definition.Adding the
disabledproperty toTabClassMapimproves type safety for all tab variants.
66-71: Fix operator precedence bug in active theme condition.Line 68 has the same critical logic error as the other variants. The condition
(isActiveTheme && tabClassType) === 'active'incorrectly evaluates the AND operation before comparing to'active'.Apply this diff to fix the condition:
return useMergeClasses([ pillsTabClasses[tabClassType], - (isActiveTheme && tabClassType) === 'active' + isActiveTheme && tabClassType === 'active' ? `${theme.backgroundClasses.value} text-white` : '', ])Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores