fix(templates): don't show section when all items are hidden #2219
Conversation
📝 WalkthroughWalkthroughThe changes update the rendering logic in multiple files by modifying the conditional check in the Changes
Sequence Diagram(s)sequenceDiagram
participant Section as Section Component
participant Filter as Item Filter
participant Render as Render Decision
Section->>Filter: Filter section.items by item.visible
Filter-->>Section: Return list of visible items
Section->>Render: Check if section is visible and list length > 0
Render-->>Section: Return decision (render or null)
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/artboard/src/templates/chikorita.tsx (1)
185-185: LGTM! Consider reducing code duplication across templates.The visibility check implementation is correct and consistent with other templates.
Consider moving the
Sectioncomponent to a shared location since it's identical across all templates. This would:
- Reduce code duplication
- Make future maintenance easier
- Ensure consistency in behavior
Example structure:
// src/components/section.tsx export const Section = <T,>({ section, children, ...props }: SectionProps<T>) => { if (!section.visible || section.items.filter((item) => item.visible).length === 0) return null; // ... rest of the implementation }; // templates/onyx.tsx, rhyhorn.tsx, etc. import { Section } from "../components/section";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/artboard/src/templates/azurill.tsx(1 hunks)apps/artboard/src/templates/bronzor.tsx(1 hunks)apps/artboard/src/templates/chikorita.tsx(1 hunks)apps/artboard/src/templates/ditto.tsx(1 hunks)apps/artboard/src/templates/gengar.tsx(1 hunks)apps/artboard/src/templates/glalie.tsx(1 hunks)apps/artboard/src/templates/kakuna.tsx(1 hunks)apps/artboard/src/templates/leafish.tsx(1 hunks)apps/artboard/src/templates/nosepass.tsx(1 hunks)apps/artboard/src/templates/onyx.tsx(1 hunks)apps/artboard/src/templates/pikachu.tsx(1 hunks)apps/artboard/src/templates/rhyhorn.tsx(1 hunks)
🔇 Additional comments (11)
apps/artboard/src/templates/kakuna.tsx (1)
199-199: LGTM! Improved section visibility logic.The updated condition correctly filters out hidden items before checking the section's length, ensuring sections with only hidden items are not displayed. This change aligns well with the PR objective.
apps/artboard/src/templates/leafish.tsx (1)
194-194: LGTM! Consistent implementation across templates.The visibility check update matches the implementation in other templates, maintaining consistency in how hidden items are handled.
apps/artboard/src/templates/azurill.tsx (1)
189-189: LGTM! Maintains consistent behavior.The visibility check update aligns with other templates, ensuring consistent handling of hidden items across the application.
apps/artboard/src/templates/bronzor.tsx (1)
180-180: LGTM! Completes the consistent implementation.The visibility check update matches other templates, completing the consistent implementation of this feature across all templates.
apps/artboard/src/templates/onyx.tsx (1)
200-200: LGTM! The visibility check enhancement improves the rendering logic.The updated condition ensures that sections with only hidden items are not rendered, which aligns with the PR objective and improves the user experience.
apps/artboard/src/templates/rhyhorn.tsx (1)
181-181: LGTM! Consistent implementation across templates.The visibility check matches the implementation in other templates, maintaining consistency in the codebase.
apps/artboard/src/templates/nosepass.tsx (1)
183-183: LGTM! Visibility check is consistent with other templates.The implementation maintains consistency with other templates while improving section visibility logic.
apps/artboard/src/templates/glalie.tsx (1)
195-195: LGTM! The visibility check is now more consistent and efficient.The change improves the component by:
- Moving the visibility check earlier in the component lifecycle
- Ensuring consistency with the filtering in the render method
- Preventing unnecessary component initialization when no items are visible
apps/artboard/src/templates/gengar.tsx (1)
187-187: LGTM! The visibility check is consistent with other templates.The change ensures consistent behavior across templates by applying the same visibility logic improvement.
apps/artboard/src/templates/pikachu.tsx (1)
215-215: LGTM! The visibility check is consistent with other templates.The change ensures consistent behavior across templates by applying the same visibility logic improvement.
apps/artboard/src/templates/ditto.tsx (1)
200-200: LGTM! The visibility check is consistent with other templates.The change ensures consistent behavior across templates by applying the same visibility logic improvement.
|
@mtdvlpr isn't would be better to move it into |
Summary by CodeRabbit