Added collapsible feature for each section #2426
Open
+161
−87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughAdds per-section collapse/expand state and UI to the resume builder sidebars: store fields and toggles for collapsed sections, header buttons with caret icons, and AnimatePresence/motion-wrapped content that conditionally renders (including DnD and item actions) when expanded. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header as Section Header
participant Store as ResumeStore
participant Content as Section Content (animated)
User->>Header: Click collapse/expand button
Header->>Store: toggleSectionCollapsed(sectionId)
Store->>Store: Update collapsedSections[sectionId]
Store-->>Header: New collapsed state
Header->>Content: Re-render (AnimatePresence/motion)
alt collapsed = false
Note right of Content: Animate in\nrender DnD, items, actions
else collapsed = true
Note right of Content: Animate out\nhide content
end
Content-->>User: Updated UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 1
🧹 Nitpick comments (8)
apps/client/src/stores/resume.ts (2)
27-29: Tighten types for collapse API to SectionKeyUse SectionKey instead of string for stronger guarantees and safer call sites.
- collapsedSections: Record<string, boolean>; - toggleSectionCollapsed: (id: string) => void; - setSectionCollapsed: (id: string, collapsed: boolean) => void; + collapsedSections: Partial<Record<SectionKey, boolean>>; + toggleSectionCollapsed: (id: SectionKey) => void; + setSectionCollapsed: (id: SectionKey, collapsed: boolean) => void;
76-88: Collapse state UX polish: initialize explicitly, clean up on delete, and (optionally) persist
- Explicit boolean cast avoids relying on undefined truthiness.
- Remove stale keys when a custom section is deleted.
- Optional: persist to localStorage so collapsed state survives reloads.
collapsedSections: {}, toggleSectionCollapsed: (id) => { set((state) => { - state.collapsedSections[id] = !state.collapsedSections[id]; + state.collapsedSections[id] = !Boolean(state.collapsedSections[id]); }); }, setSectionCollapsed: (id, collapsed) => { set((state) => { state.collapsedSections[id] = collapsed; }); },Additionally, in removeSection (outside this hunk), delete the entry:
set((state) => { removeItemInLayout(sectionId, state.resume.data.metadata.layout); // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete state.resume.data.sections.custom[id]; + // Clear any UI state tied to this section + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete state.collapsedSections[sectionId as unknown as string];Optional persistence (can be a follow-up): wrap the store with persist and partialize to only keep collapsedSections.
// import { persist } from "zustand/middleware"; // create(persist(temporal(immer(...)), { name: "resume-ui", partialize: (s) => ({ collapsedSections: s.collapsedSections }) }))apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx (3)
41-47: Minor: add a stable contentId and avoid unchecked cast where possible
- Define contentId once for a11y wiring with aria-controls.
- The SectionWithItem cast is unchecked; if feasible, narrow via a type guard to reduce unsafe casts. Optional.
- const collapsed = useResumeStore((state) => state.collapsedSections[id] ?? false); - const toggleSectionCollapse = useResumeStore((state) => state.toggleSectionCollapsed); + const collapsed = useResumeStore((state) => state.collapsedSections[id] ?? false); + const toggleSectionCollapse = useResumeStore((state) => state.toggleSectionCollapsed); + const contentId = `${id}-content`; const setValue = useResumeStore((state) => state.setValue); const section = useResumeStore( (state) => get(state.resume.data.sections, id) as SectionWithItem<T>, );
103-113: Add button type and ARIA to improve accessibilityPrevent implicit form submit and expose state to AT via aria-expanded/controls.
- <button - className="text-gray-500 transition-colors hover:text-gray-700" - aria-label={collapsed ? t`Expand section` : t`Collapse section`} + <button + type="button" + className="text-gray-500 transition-colors hover:text-gray-700 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/50" + aria-label={collapsed ? t`Expand section` : t`Collapse section`} + aria-expanded={!collapsed} + aria-controls={contentId} onClick={() => { toggleSectionCollapse(id); }} >
123-207: Consistent motion: avoid initial mount jump and wire aria-controls targetUse initial={false} to skip first-mount animation and give the content an id to match aria-controls.
- <AnimatePresence initial={false}> + <AnimatePresence initial={false}> {!collapsed && ( <motion.div - key="content" + key="content" + id={contentId} initial={{ height: 0, opacity: 0 }} animate={{ height: "auto", opacity: 1 }} exit={{ height: 0, opacity: 0 }} - transition={{ duration: 0.25, ease: "easeInOut" }} + transition={{ duration: 0.25, ease: "easeInOut" }} className="overflow-hidden" >apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (3)
28-36: A11y: button type and ARIA wiringAdd type="button", aria-expanded, and aria-controls. Also define a stable content id.
- <div className="flex items-center gap-x-4"> - <button + <div className="flex items-center gap-x-4"> + <button + type="button" className="text-gray-500 transition-colors hover:text-gray-700" aria-label={collapsed ? t`Expand section` : t`Collapse section`} + aria-expanded={!collapsed} + aria-controls="summary-content" onClick={() => { - toogleSectionCollapse("summary"); + toggleSectionCollapse("summary"); }} >
46-74: Align motion behavior and connect aria-controlsSkip first-mount animation and give the content an id to match aria-controls. Consider matching duration/easing with SectionBase for consistency.
- <AnimatePresence> + <AnimatePresence initial={false}> {!collapsed && ( <motion.div - key="summary-content" + key="summary-content" + id="summary-content" initial={{ opacity: 0, height: 0 }} - animate={{ opacity: 1, height: "auto" }} + animate={{ opacity: 1, height: "auto" }} exit={{ opacity: 0, height: 0 }} - transition={{ duration: 0.2 }} + transition={{ duration: 0.25, ease: "easeInOut" }} >
55-71: Avoid double updates to store when AI footer writes contentAiActions footer sets content via editor and also calls setValue, while RichInput onChange also calls setValue. This likely triggers two updates per action. Prefer a single source:
Option A: keep RichInput onChange; in footer, only update editor content.
- footer={(editor) => ( + footer={(editor) => ( <AiActions value={editor.getText()} onChange={(value) => { editor.commands.setContent(value, true); - setValue("sections.summary.content", value); }} /> )}Option B: if keeping footer setValue, remove the outer onChange to prevent duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx(4 hunks)apps/client/src/pages/builder/sidebars/left/sections/summary.tsx(3 hunks)apps/client/src/stores/resume.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (2)
apps/client/src/stores/resume.ts (1)
useResumeStore(32-96)apps/client/src/components/ai-actions.tsx (1)
AiActions(36-134)
apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx (2)
apps/client/src/stores/resume.ts (1)
useResumeStore(32-96)apps/client/src/pages/builder/sidebars/left/sections/shared/section-list-item.tsx (1)
SectionListItem(34-112)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (3)
58-66: Avoid double store updates from AI footer.setContent(true) already triggers RichInput.onUpdate → setValue; remove the extra setValue to prevent duplicate debounced writes.
- <AiActions - value={editor.getText()} - onChange={(value) => { - editor.commands.setContent(value, true); - setValue("sections.summary.content", value); - }} - /> + <AiActions + value={editor.getText()} + onChange={(value) => { + editor.commands.setContent(value, true); // onUpdate will persist HTML + }} + />Also applies to: 67-70
46-54: Smoother height animation and no first-render pop.Hide overflow during height tween and skip initial mount animation.
- <AnimatePresence> + <AnimatePresence initial={false}> {!collapsed && ( - <motion.div + <motion.div + id="summary-panel" + className="overflow-hidden" key="summary-content" initial={{ opacity: 0, height: 0 }} animate={{ opacity: 1, height: "auto" }} exit={{ opacity: 0, height: 0 }} transition={{ duration: 0.2 }} >
14-22: Minor polish: de-duplicate literals and consider persisting UI state.
- Define a single const id = "summary" to avoid repeating the literal.
- Consider persisting collapsedSections via a small UI store (zustand persist) so collapse survives reloads; keep it out of temporal history if desired.
If helpful, I can draft a tiny ui/store with persist that only keeps collapsedSections.
Also applies to: 28-36, 41-43, 46-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (3)
apps/client/src/stores/resume.ts (1)
useResumeStore(32-96)libs/ui/src/components/rich-input.tsx (1)
RichInput(477-531)apps/client/src/components/ai-actions.tsx (1)
AiActions(36-134)
Comment on lines
+28
to
+36
| <button | ||
| className="text-gray-500 transition-colors hover:text-gray-700" | ||
| aria-label={collapsed ? t`Expand section` : t`Collapse section`} | ||
| onClick={() => { | ||
| toggleSectionCollapse("summary"); | ||
| }} | ||
| > | ||
| {collapsed ? <CaretRightIcon size={18} /> : <CaretDownIcon size={18} />} | ||
| </button> |
There was a problem hiding this comment.
Add disclosure a11y attributes (and set type).
Expose state to assistive tech and prevent implicit submit. Add aria-expanded/aria-controls and type="button".
- <button
- className="text-gray-500 transition-colors hover:text-gray-700"
- aria-label={collapsed ? t`Expand section` : t`Collapse section`}
- onClick={() => {
+ <button
+ type="button"
+ className="text-gray-500 transition-colors hover:text-gray-700"
+ aria-label={collapsed ? t`Expand section` : t`Collapse section`}
+ aria-expanded={!collapsed}
+ aria-controls="summary-panel"
+ onClick={() => {
toggleSectionCollapse("summary");
}}
>
{collapsed ? <CaretRightIcon size={18} /> : <CaretDownIcon size={18} />}
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| className="text-gray-500 transition-colors hover:text-gray-700" | |
| aria-label={collapsed ? t`Expand section` : t`Collapse section`} | |
| onClick={() => { | |
| toggleSectionCollapse("summary"); | |
| }} | |
| > | |
| {collapsed ? <CaretRightIcon size={18} /> : <CaretDownIcon size={18} />} | |
| </button> | |
| <button | |
| type="button" | |
| className="text-gray-500 transition-colors hover:text-gray-700" | |
| aria-label={collapsed ? t`Expand section` : t`Collapse section`} | |
| aria-expanded={!collapsed} | |
| aria-controls="summary-panel" | |
| onClick={() => { | |
| toggleSectionCollapse("summary"); | |
| }} | |
| > | |
| {collapsed ? <CaretRightIcon size={18} /> : <CaretDownIcon size={18} />} | |
| </button> |
🤖 Prompt for AI Agents
In apps/client/src/pages/builder/sidebars/left/sections/summary.tsx around lines
28–36, the collapse toggle button is missing disclosure attributes and a type,
so update the button to include type="button", aria-expanded={collapsed ?
"false" : "true"}, and aria-controls pointing to the id of the collapsible
content (eg. "summary-section" or "summary-panel"); also ensure the collapsible
content element has the matching id and appropriate hidden/aria-hidden state so
assistive tech can detect the panel state.
Comment on lines
+55
to
+71
| <main className={cn(!section.visible && "opacity-50")}> | ||
| <RichInput | ||
| content={section.content} | ||
| footer={(editor) => ( | ||
| <AiActions | ||
| value={editor.getText()} | ||
| onChange={(value) => { | ||
| editor.commands.setContent(value, true); | ||
| setValue("sections.summary.content", value); | ||
| }} | ||
| /> | ||
| )} | ||
| onChange={(value) => { | ||
| setValue("sections.summary.content", value); | ||
| }} | ||
| /> | ||
| </main> |
There was a problem hiding this comment.
Use
, not , inside a reusable section.
Only one landmark
per page. Replace with a non-landmark container.- <main className={cn(!section.visible && "opacity-50")}>
+ <div className={cn(!section.visible && "opacity-50")}>
<RichInput
content={section.content}
footer={(editor) => (
<AiActions
value={editor.getText()}
onChange={(value) => {
editor.commands.setContent(value, true);
- setValue("sections.summary.content", value);
}}
/>
)}
onChange={(value) => {
setValue("sections.summary.content", value);
}}
/>
- </main>
+ </div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/pages/builder/sidebars/left/sections/summary.tsx around lines
55 to 71, the component uses a <main> element inside a reusable section which
violates the single-page landmark rule; replace the <main> element with a
non-landmark container (e.g., <div>) preserving the existing className and
children, and if you need to keep semantic meaning for assistive tech add an
appropriate ARIA role or aria-label on the div; ensure no other code expects a
<main> tag so tests/types remain valid.
|
Please someone review it. |
|
Nice! Definitely enhances the user experience! Thanks for adding this feature. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added features
Resolves #2398
Demo
Summary by CodeRabbit