Feature: improved Accordion component flexibility #351
Conversation
Sqrcz
added
📚 documentation
WalkthroughThe changes update the accordion documentation and component implementation. In the docs, example imports and section headers have been revised for clarity; several outdated examples have been removed in favor of new ones with descriptive content. In the source, component files and composables are refactored with renamed variables, updated props, and improved state management. The type definitions have been overhauled, replacing older type aliases with structured interfaces, ensuring consistency across the accordion components. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant H as AccordionHeader
participant S as AccordionState Manager
participant P as AccordionPanel
participant C as AccordionContent
U->>H: Click to toggle panel
H->>S: Request current state
S-->>H: Return panel state
H->>P: Toggle panel visibility
P->>C: Update content classes (via composable)
C-->>U: Render updated accordion
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/components/FwbAccordion/composables/useAccordionContentClasses.ts (2)
12-12: Consider externalizing base class
Hardcoding thebaseContentClassesstring can be fine, but consider moving it to a shared constants file or a configurable theme object for easier maintenance.
22-35: Refine merged class order
WhileuseMergeClasseshelps unify class strings, consider clarifying prioritization rules (e.g., user classes taking precedence). This ensures consistent outcomes when classes overlap or conflict.src/components/FwbAccordion/composables/useAccordionHeaderClasses.ts (3)
7-9: Consider consolidating common classes.
commonHeaderClasses,baseHeaderClasses, andflushedHeaderClassesshare many similar Tailwind classes. Consider extracting shared segments (e.g.,flex w-full items-center justify-between) to avoid duplication and ease future maintenance.-const commonHeaderClasses = 'flex w-full items-center justify-between gap-3 font-medium p-5 text-gray-500 dark:text-gray-400 rtl:text-right' -const baseHeaderClasses = `${commonHeaderClasses} border border-gray-200 hover:bg-gray-100 focus:ring-4` -const flushedHeaderClasses = `${commonHeaderClasses} border-b border-gray-200` +const sharedHeaderClasses = 'flex w-full items-center justify-between gap-3 font-medium rtl:text-right' +const baseHeaderClasses = `${sharedHeaderClasses} p-5 text-gray-500 dark:text-gray-400 border border-gray-200 hover:bg-gray-100 focus:ring-4` +const flushedHeaderClasses = `${sharedHeaderClasses} p-5 text-gray-500 dark:text-gray-400 border-b border-gray-200`
39-54: Use offilter(str => (str))is fine, butfilter(Boolean)is more idiomatic.Tailwind class arrays often use
filter(Boolean)for cleaner code. If you prefer brevity and clarity, consider replacing the current filter logic:-].filter(str => (str)) +.filter(Boolean)
50-50: Consider unifying user-defined active classes.
isPanelVisible.value ? userActiveClass.value : ''is valid, yet for greater flexibility, you might also allow custom classes when not active. This would align with the approach used inuseAccordionPanelClasseswhere you merge empty classes otherwise.src/components/FwbAccordion/FwbAccordionContent.vue (1)
30-38: Consider memoizing the contentClasses computation.The current implementation recomputes classes on every render. Consider using
useMemoor extracting static parts to improve performance.-const contentClasses = computed(() => (accordionState.value && panelState.value) - ? useAccordionContentClasses(accordionState, panelState, props) - : null) +const contentClasses = computed(() => { + if (!accordionState.value || !panelState.value) return null + // Memoize the static parts of the classes + const baseClasses = useAccordionContentClasses(accordionState, panelState, props) + return baseClasses +})docs/components/accordion/examples/FwbAccordionExampleStyling.vue (1)
12-17: Consider adding aria-labels for accessibility.While the styling is well-implemented, consider enhancing accessibility.
<fwb-accordion-header class="w-full justify-center rounded-none border-0 bg-white hover:bg-cyan-200 dark:bg-cyan-800 dark:text-white dark:hover:bg-cyan-700 [&>svg]:m-0" :active-class="'bg-lime-200 hover:bg-lime-300 dark:bg-lime-500 dark:text-black dark:hover:bg-lime-400'" + :aria-label="`Toggle item ${i+1}`" >src/components/FwbAccordion/FwbAccordionPanel.vue (1)
73-77: Consider adding error handling for edge cases.The panel state initialization could benefit from additional error handling.
accordionState.value.panels.push({ id: panelId, - isVisible: (!accordionState.value.collapsed && panelIndex === 0) ?? false, + isVisible: accordionState.value?.collapsed === false && panelIndex === 0 || false, order: panelIndex, })docs/components/accordion.md (2)
81-81: Add language specifier to code block.Add a language specifier to the fenced code block for better syntax highlighting.
-``` +```vue🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
81-81: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
325-352: Add blank lines around tables in API section.Add blank lines before and after tables to improve markdown formatting.
Example format:
### FwbAccordion Props | Name | Type | Default | | ----- | ---------------- | ------- | | class | String \| Object | `''` | ### FwbAccordionPanel Props🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
326-326: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
331-331: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
337-337: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
343-343: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
349-349: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/components/accordion.md(6 hunks)docs/components/accordion/examples/FwbAccordionExample.vue(1 hunks)docs/components/accordion/examples/FwbAccordionExampleAlwaysOpen.vue(0 hunks)docs/components/accordion/examples/FwbAccordionExampleCollapsed.vue(1 hunks)docs/components/accordion/examples/FwbAccordionExampleFirstItemClosed.vue(0 hunks)docs/components/accordion/examples/FwbAccordionExamplePersistent.vue(1 hunks)docs/components/accordion/examples/FwbAccordionExampleStyledHeaders.vue(0 hunks)docs/components/accordion/examples/FwbAccordionExampleStyling.vue(1 hunks)src/components/FwbAccordion/FwbAccordion.vue(1 hunks)src/components/FwbAccordion/FwbAccordionContent.vue(2 hunks)src/components/FwbAccordion/FwbAccordionHeader.vue(1 hunks)src/components/FwbAccordion/FwbAccordionPanel.vue(1 hunks)src/components/FwbAccordion/composables/useAccordionContentClasses.ts(1 hunks)src/components/FwbAccordion/composables/useAccordionHeaderClasses.ts(1 hunks)src/components/FwbAccordion/composables/useAccordionPanelClasses.ts(1 hunks)src/components/FwbAccordion/composables/useAccordionState.ts(1 hunks)src/components/FwbAccordion/types.ts(1 hunks)
💤 Files with no reviewable changes (3)
- docs/components/accordion/examples/FwbAccordionExampleFirstItemClosed.vue
- docs/components/accordion/examples/FwbAccordionExampleAlwaysOpen.vue
- docs/components/accordion/examples/FwbAccordionExampleStyledHeaders.vue
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/accordion.md
81-81: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
326-326: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
331-331: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
337-337: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
343-343: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
349-349: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
🔇 Additional comments (46)
src/components/FwbAccordion/composables/useAccordionContentClasses.ts (6)
3-3: Imports look good
No immediate issues with importing these specific types.
5-5: Use ofuseMergeClassesis appropriate
ImportinguseMergeClassesaligns well with constructing dynamic class strings.
7-10: Function signature is clear
The updated signature explicitly receivesaccordionState,panelState, andprops, improving clarity compared to a singlecontentRef. Ensure downstream usage is updated accordingly.
14-15: Good approach to user-defined classes
Using computed properties forprops.classandprops.activeClasscleanly handles dynamic user styling.
17-21: Check panel indexing logic
Be sure the indexing logic accurately handles edge cases (e.g., whenpanelState.valueisnull). The computations look correct otherwise.
37-37: Return value is well-structured
Returning the computed class string is straightforward and aligns with Vue best practices.src/components/FwbAccordion/composables/useAccordionState.ts (11)
3-9: New type imports
Importing the refined types from the shared file centralizes type management effectively.
11-11: Reactive state initialization
Creating a sharedaccordionStatesreactive object is a practical way to track multiple accordions.
13-17: Clear function contract
The function now returns objects for retrieving accordion states, helpful for consistent state access across components.
19-20: Edge case for missing ID
Skipping state setup whenaccordionIdis falsy prevents errors but verify if the calling component can genuinely operate without an ID.
21-25: State creation block
Property defaults (collapsed,flushed,persistent) are well-defined. This block is neatly structured.
29-32: Cleanly removing state on unmount
Deleting the accordion state on unmount prevents memory leaks. The dynamic delete approach is acceptable.
35-39:getAccordionStatelogic
Determining the nearest parent[data-accordion-id]is a neat way to locate the relevant accordion. Make sure DOM-based approach is valid for SSR if needed.
41-43:getAccordionPanelStateapproach
Querying thepanelsarray by matchingpanelIdis straightforward. Confirm thatpanelIduniqueness is enforced at the calling site.
45-47: Final return structure
ExposingaccordionStates,getAccordionState, andgetAccordionPanelStateprovides a cohesive API for broader usage.
50-50: No additional code
No changes made here; no issues.
51-53: Export statement
Re-exporting the composable from a single file is a clean structure for outward usage.src/components/FwbAccordion/types.ts (16)
1-1: Ref import
ImportingReffor typed references is appropriate.
3-8: InterfaceAccordionProps
Optional attributes (class,collapsed,flushed,persistent) are well-suited for flexible accordion configuration.
10-10:AccordionStateextendsAccordionProps
Allows reusing shared prop fields while addingidandpanels; this is an efficient architecture.
12-13:panels: AccordionPanel[]
Excellent approach for storing multiple panel states.
15-16:AccordionStatesindexed by ID
A dictionary of states keyed by string IDs is clear and scalable for multiple accordions.
19-20:AccordionPanelProps
OptionalactiveClassfor panel styling is consistent with the pattern set inAccordionProps.
23-23: RefinedAccordionPanel
Definingid,isVisible, andordercovers essential panel metadata.
25-27: Panel visibility and order
Ensures unambiguous ordering while exposing visibility for dynamic UI logic.
29-31:GetAccordionStateAttributes
Using an element reference to find the relevant accordion is a clever approach.
33-35:GetAccordionState
Neatly represents a function accessor to retrieve the entire accordion state by DOM reference.
37-40:GetAccordionPanelStateAttributes
Separating outaccordionStatefrompanelIdclarifies usage for retrieving specific panel data.
42-43:GetAccordionPanelState
This function signature is consistent: purely typed, returning the panel ornull.
46-51:UseAccordionState
Defining a higher-level interface for the composable’s return values centralizes the state API.
54-57:AccordionHeaderProps
OptionalactiveClassandclassfor headers allow consistent styling parallels with other props.
59-61:AccordionContentProps
Maintaining a similar pattern of optional styling props fosters consistency throughout.
64-75: Consolidated export
Collecting all related types in a single export statement is a neat, maintainable approach.src/components/FwbAccordion/composables/useAccordionHeaderClasses.ts (2)
5-5: Good pattern for merging classes.Importing and using a dedicated
useMergeClassescomposable clarifies class handling and maintains consistency across the codebase.
23-28: Rotate arrow logic is concise.The conditional addition of
'rotate-180'whenpanelState.value?.isVisibleistrueis straightforward and improves readability.src/components/FwbAccordion/composables/useAccordionPanelClasses.ts (1)
10-14: Straightforward panel class composable.Merging only the active class when
isVisibleistrueis clear and maintains minimal overhead. If you anticipate needing more systematic merging (e.g., default vs. custom classes), consider expanding to a pattern similar touseAccordionHeaderClasses.src/components/FwbAccordion/FwbAccordion.vue (3)
2-5: Effective use of dynamic class binding.Binding
:class="accordionClasses"leverages computed properties and clarifies how user-defined classes combine with defaults.
21-28: Good defaults for new props.Defining defaults (
'',false) keeps the component more predictable and ensures minimal breakage if a parent omits props.
30-36: Encapsulation and reusability.Using
useMergeClassesto combineaccordionDefaultClassesanduserClassesfosters consistency and simplifies the approach to styling. This pattern can be easily extended for additional theming or customization in the future.src/components/FwbAccordion/FwbAccordionContent.vue (1)
20-25: LGTM! Props implementation aligns with PR objectives.The implementation of
activeClassandclassprops with default empty strings provides good flexibility for custom styling.docs/components/accordion/examples/FwbAccordionExampleStyling.vue (1)
4-11: LGTM! Excellent demonstration of panel styling and events.The implementation effectively showcases:
- Dynamic class binding for border styles
- Event handling for panel visibility
- Clean state management
src/components/FwbAccordion/FwbAccordionPanel.vue (1)
51-62: LGTM! Clean implementation of show/hide events.The event implementation is well-structured and properly typed.
src/components/FwbAccordion/FwbAccordionHeader.vue (1)
61-73: LGTM! Well-structured panel toggling logic.The implementation properly handles:
- Persistent panel state
- Mutually exclusive panel visibility
- State transitions
docs/components/accordion/examples/FwbAccordionExample.vue (1)
1-72: LGTM! Content improvements enhance readability.The changes improve the documentation by:
- Using descriptive headers that better explain the content
- Structuring content with semantic HTML
- Maintaining consistent formatting
docs/components/accordion/examples/FwbAccordionExamplePersistent.vue (1)
1-67: LGTM! Well-structured example for persistent state.The example correctly demonstrates the
persistentprop usage with consistent content structure.docs/components/accordion.md (1)
276-278: LGTM! Comprehensive API documentation.The new API section provides clear documentation of props and events, aligning with the PR objectives of supporting custom styling and visibility events.
| @@ -0,0 +1,67 @@ | |||
| <template> | |||
| <div class="vp-raw"> | |||
| <fwb-accordion collapsed> | |||
There was a problem hiding this comment.
Fix prop inconsistency in the example code.
The example uses collapsed prop in the component (line 3) but shows persistent in the documentation code (line 217).
Apply this diff to fix the documentation code:
- <fwb-accordion persistent>
+ <fwb-accordion collapsed>Also applies to: 217-217
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/components/accordion/examples/FwbAccordionExampleFlushed.vue (1)
3-61: Enhance accessibility with ARIA labels.Consider adding descriptive ARIA labels to improve accessibility for screen readers.
Apply this diff to enhance accessibility:
- <fwb-accordion flushed> + <fwb-accordion flushed aria-label="Flowbite FAQ"> <fwb-accordion-panel> - <fwb-accordion-header>What is Flowbite?</fwb-accordion-header> + <fwb-accordion-header aria-level="3">What is Flowbite?</fwb-accordion-header>docs/components/accordion.md (2)
325-329: Add blank lines around tables and include prop descriptions.The table formatting needs improvement, and prop descriptions would enhance documentation clarity.
Apply this diff:
## Accordion components API ### FwbAccordion Props + | Name | Type | Default | Description | | ----- | ---------------- | ------- | ----------- | -| class | String \| Object | `''` | +| class | String \| Object | `''` | Custom CSS classes to apply to the accordion | +🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
326-326: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
81-81: Add language specifier to code block.Code blocks should specify their language for proper syntax highlighting.
Add language specifier to the code block:
-``` +```js🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
81-81: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/components/accordion.md(6 hunks)docs/components/accordion/examples/FwbAccordionExampleFlush.vue(0 hunks)docs/components/accordion/examples/FwbAccordionExampleFlushed.vue(1 hunks)
💤 Files with no reviewable changes (1)
- docs/components/accordion/examples/FwbAccordionExampleFlush.vue
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/accordion.md
81-81: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
326-326: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
331-331: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
337-337: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
343-343: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
349-349: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
🔇 Additional comments (1)
docs/components/accordion.md (1)
276-278: LGTM! Clear documentation of new features.The documentation clearly explains the new styling capabilities and visibility events, which aligns well with the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/components/accordion.md (2)
210-214: Typographical Correction Needed:
The heading "Initialy Collapsed" contains a minor typo. Consider renaming it to "Initially Collapsed" for clarity and consistency.
322-352: Accordion Components API Documentation:
The API documentation tables forFwbAccordion,FwbAccordionPanel,FwbAccordionHeader, andFwbAccordionContentare thorough and provide essential information about props and events.
Suggestion: For improved Markdown readability and to comply with best practices (e.g., markdownlint MD058), consider surrounding the tables with blank lines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
325-325: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
330-330: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
336-336: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
342-342: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
348-348: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/components/accordion.md(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/components/accordion.md
80-80: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
325-325: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
330-330: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
336-336: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
342-342: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
348-348: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
🔇 Additional comments (9)
docs/components/accordion.md (9)
1-7: New Example Component Imports:
The new imports forFwbAccordionExampleCollapsed,FwbAccordionExampleFlushed,FwbAccordionExamplePersistent, andFwbAccordionExampleStylingcorrectly extend the documentation examples in line with the new accordion flexibility feature. Please verify that these file paths are correct and that the components exist.
9-20: Basic Accordion Section Update:
The "Basic Accordion" section header and supporting description are clear and concise. The example usage introduced here maintains consistency with the overall documentation style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
82-86: Persistent Accordion Items Heading:
The header and introductory text for the "Persistent Accordion Items" section correctly explain that multiple accordion items can be open simultaneously with thepersistentprop.
85-144: Persistent Accordion Example Code:
The Vue code snippet clearly demonstrates using thepersistentprop on<fwb-accordion>, allowing multiple panels to remain open. The structure and formatting effectively showcase the feature.
146-150: Flushed Accordion Section Description:
The introduction of the "Flushed Accordion" section is clear. The description of theflushedprop and its visual effect aligns well with the updated design requirements.
151-208: Flushed Accordion Example Code:
The Vue template for the flushed accordion example succinctly demonstrates the removal of extraneous styling (background color, side borders, and rounded corners) via theflushedprop. The code is readable and effectively illustrates the intended design.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
208-208: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
215-273: Collapsed Accordion Example Code:
The code snippet properly employs thecollapsedprop on<fwb-accordion>, ensuring that the accordion is fully collapsed by default. The structure and content clearly demonstrate the intended behavior.
275-278: Styling Accordions Section Introduction:
The section introducing the usage of dedicatedclassandactiveClassprops, along with the newshowandhideevents, is well articulated. The explanation provides a clear rationale for the enhanced styling flexibility.
281-320: Advanced Styling Example Code:
The Vue template leverages av-forloop to render multiple accordion panels, and it uses conditional class bindings alongside@showand@hideevent handlers to update panel visibility dynamically. This implementation effectively demonstrates the advanced styling and interactivity features.
This PR enhances the Accordion component with:
classandactiveClassprops onFwbAccordion,FwbAccordionPanel,FwbAccordionHeader, andFwbAccordionContent.showandhideevents emitted by theFwbAccordionPanelcomponent when panel visibility changes.always-openis nowpersistentflushis nowflushed:open-first-item="false"is nowcollapsedSummary by CodeRabbit
Documentation
New Features
Refactor