feat(widget): add collapsible split view for responsive dashboard#548
feat(widget): add collapsible split view for responsive dashboard#548sandy-yield wants to merge 3 commits into
Conversation
Collapse the dashboard's two-column layout into a single panel at/below 800px, with an edge toggle bar (sitting on the hidden side) that swaps between panels while keeping both mounted to preserve state. Adds a useMediaQuery hook and shared split breakpoint helpers, scopes the column max-widths to desktop, and animates the panel transition.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a responsive dashboard split-view layout with a shared media-query hook, a SplitView component, updated dashboard shell styling, and overview/position-details pages that render split panes with translated labels and responsive width rules. ChangesDashboard split-view layout
Sequence Diagram(s)sequenceDiagram
participant User
participant SplitView
participant useMediaQuery
participant "window.matchMedia" as MatchMedia
participant MediaQueryList
User->>SplitView: render page content
SplitView->>useMediaQuery: useMediaQuery(splitCollapsedMediaQuery)
useMediaQuery->>MatchMedia: matchMedia(query)
MatchMedia-->>useMediaQuery: MediaQueryList
useMediaQuery->>MediaQueryList: addEventListener("change", ...)
User->>SplitView: click collapsed bar
SplitView->>SplitView: setActiveSide(...)
MediaQueryList-->>useMediaQuery: change event
SplitView-->>User: update visible pane
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/widget/src/pages-dashboard/common/components/tab-page-container.tsx (1)
1-14: 📐 Maintainability & Code Quality | 🟡 MinorDelete
packages/widget/src/pages-dashboard/common/components/tab-page-container.tsx. The entireTabPageContainerimplementation is commented out, and there are no remaining references toTabPageContainerortab-page-containerelsewhere in the component tree. Remove the dead file instead of keeping commented-out code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages-dashboard/common/components/tab-page-container.tsx` around lines 1 - 14, Remove the dead TabPageContainer module entirely: the entire implementation in TabPageContainer is commented out and there are no remaining references to TabPageContainer or tab-page-container. Delete the file rather than leaving a commented stub, and ensure any imports or exports that may still reference TabPageContainer are cleaned up.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widget/src/pages-dashboard/common/components/split-view/index.tsx`:
- Around line 28-30: The pane-presence check in split-view is using a falsy test
that can treat valid ReactNode values like 0 or "" as missing. Update the
conditional in the SplitView render logic to use explicit null/undefined checks
for primary and secondary instead of !primary or !secondary, and keep the
fallback Box rendering behavior the same when one pane is actually absent.
In `@packages/widget/src/pages-dashboard/common/components/styles.css.ts`:
- Around line 14-16: Remove the conflicting minWidth rule from the dashboard
styles so the maxWidth cap can work correctly. Update the style object in the
shared dashboard component styles to keep width on 100% and maxWidth at 1000px,
but drop minWidth from the same declaration. Use the existing styles definition
in the component stylesheet to locate and adjust this layout rule.
In `@packages/widget/src/styles/tokens/breakpoints.ts`:
- Around line 24-25: The split/collapse breakpoint logic in
splitExpandedMediaQuery creates a fractional no-match gap between the collapsed
and expanded queries. Update the breakpoint definitions around
SPLIT_COLLAPSE_BREAKPOINT and minMediaQuery so the collapsed and expanded ranges
meet without leaving any unmatched widths, using the existing breakpoint symbols
in breakpoints.ts.
---
Outside diff comments:
In
`@packages/widget/src/pages-dashboard/common/components/tab-page-container.tsx`:
- Around line 1-14: Remove the dead TabPageContainer module entirely: the entire
implementation in TabPageContainer is commented out and there are no remaining
references to TabPageContainer or tab-page-container. Delete the file rather
than leaving a commented stub, and ensure any imports or exports that may still
reference TabPageContainer are cleaned up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c4c458c-a606-474e-963e-d6623f9ba1b2
📒 Files selected for processing (14)
packages/widget/src/hooks/use-media-query.tspackages/widget/src/pages-dashboard/common/components/split-view/index.tsxpackages/widget/src/pages-dashboard/common/components/split-view/styles.css.tspackages/widget/src/pages-dashboard/common/components/styles.css.tspackages/widget/src/pages-dashboard/common/components/tab-page-container.tsxpackages/widget/src/pages-dashboard/overview/index.tsxpackages/widget/src/pages-dashboard/overview/styles.css.tspackages/widget/src/pages-dashboard/position-details/index.tsxpackages/widget/src/pages-dashboard/position-details/styles.css.tspackages/widget/src/style.css.tspackages/widget/src/styles/theme/global.css.tspackages/widget/src/styles/tokens/breakpoints.tspackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.json
| if (!primary || !secondary) { | ||
| return <Box className={styles.container}>{primary || secondary}</Box>; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use null checks instead of falsy checks for pane presence.
!primary || !secondary can misclassify valid ReactNode values like 0 or "" as missing.
Suggested fix
- if (!primary || !secondary) {
+ if (primary == null || secondary == null) {
return <Box className={styles.container}>{primary || secondary}</Box>;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/pages-dashboard/common/components/split-view/index.tsx`
around lines 28 - 30, The pane-presence check in split-view is using a falsy
test that can treat valid ReactNode values like 0 or "" as missing. Update the
conditional in the SplitView render logic to use explicit null/undefined checks
for primary and secondary instead of !primary or !secondary, and keep the
fallback Box rendering behavior the same when one pane is actually absent.
| maxWidth: "1000px", | ||
| width: "100%", | ||
| minWidth: "100%", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bwrapper\s*\(' packages/widget/src -C3
rg -nP 'minWidth' packages/widget/src/pages-dashboard/common/components/styles.css.ts -C2Repository: stakekit/widget
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'packages/widget/src/pages-dashboard/common/components/*' 'packages/widget/src/pages-dashboard/**/styles.css.ts' 'packages/widget/src/**/styles.css.ts' | sed -n '1,200p'
echo '--- styles.css.ts outline ---'
ast-grep outline packages/widget/src/pages-dashboard/common/components/styles.css.ts --view expanded || true
echo '--- surrounding file ---'
cat -n packages/widget/src/pages-dashboard/common/components/styles.css.ts | sed -n '1,120p'
echo '--- search for wrapper/appContainer/dashboard variant ---'
rg -n 'appContainer|wrapper|dashboard variant|minWidth: "100%"|maxWidth: "1000px"' packages/widget/src -C 3Repository: stakekit/widget
Length of output: 29018
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- packages/widget/src/style.css.ts ---'
cat -n packages/widget/src/style.css.ts | sed -n '1,140p'
echo '--- packages/widget/src/pages-dashboard/common/components/wrapper.tsx ---'
cat -n packages/widget/src/pages-dashboard/common/components/wrapper.tsx | sed -n '1,120p'Repository: stakekit/widget
Length of output: 3150
Remove minWidth: "100%"
minWidth: "100%" prevents the new maxWidth: "1000px" cap from taking effect on wider dashboard layouts. width: "100%" is enough here.
🐛 Proposed fix
maxWidth: "1000px",
width: "100%",
- minWidth: "100%",📝 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.
| maxWidth: "1000px", | |
| width: "100%", | |
| minWidth: "100%", | |
| maxWidth: "1000px", | |
| width: "100%", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/pages-dashboard/common/components/styles.css.ts` around
lines 14 - 16, Remove the conflicting minWidth rule from the dashboard styles so
the maxWidth cap can work correctly. Update the style object in the shared
dashboard component styles to keep width on 100% and maxWidth at 1000px, but
drop minWidth from the same declaration. Use the existing styles definition in
the component stylesheet to locate and adjust this layout rule.
| export const splitExpandedMediaQuery = minMediaQuery( | ||
| SPLIT_COLLAPSE_BREAKPOINT + 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid a no-match gap between collapsed and expanded media queries.
Using min-width: 801px leaves widths between 800px and 801px unmatched (fractional CSS pixels), which can cause inconsistent responsive behavior.
Suggested fix
export const splitExpandedMediaQuery = minMediaQuery(
- SPLIT_COLLAPSE_BREAKPOINT + 1
+ SPLIT_COLLAPSE_BREAKPOINT
);📝 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.
| export const splitExpandedMediaQuery = minMediaQuery( | |
| SPLIT_COLLAPSE_BREAKPOINT + 1 | |
| export const splitExpandedMediaQuery = minMediaQuery( | |
| SPLIT_COLLAPSE_BREAKPOINT | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/styles/tokens/breakpoints.ts` around lines 24 - 25, The
split/collapse breakpoint logic in splitExpandedMediaQuery creates a fractional
no-match gap between the collapsed and expanded queries. Update the breakpoint
definitions around SPLIT_COLLAPSE_BREAKPOINT and minMediaQuery so the collapsed
and expanded ranges meet without leaving any unmatched widths, using the
existing breakpoint symbols in breakpoints.ts.
dnehl
left a comment
dnehl
left a comment
There was a problem hiding this comment.
@petar-omni — could you take a careful look at this one based on what we discussed last week re: widget-pro? Your point then was that the responsive layout isn't really something we need to own — it's more of a showcase, and integrators would normally add their own breakpoints / widget variants and configure it how they like. The split-view itself is fine as a demo, but one thing in here goes beyond that and I'd like your read before we merge:
Global style changes that hit every embed, not just the dashboard showcase:
- padding: 16px added to appContainer base (style.css.ts) — this applies to the standalone widget variant too (App.tsx), and there was no padding before. So every existing customer integration gains 16px of padding → visible shift.
- global.css.ts adds maxWidth: 1000px + margin: auto on the root selector — forces centering/capping on all embeds.
- wrapper width 1000px → 100% / minWidth 100%.
My main concern: existing integrations should look identical after this, and these change the default appearance for everyone without opt-in.
Can we pull these out of the showcase PR (or explicitly justify them)?
Restrict the responsive split-view layout styles to dashboard mode so standalone widget integrations render identically to before. Move the root max-width/centering out of the global root selector into a dashboard-only rule gated by a data-sk-layout marker, and drop the appContainer base padding.
2 participants
Collapse the dashboard's two-column layout into a single panel at/below 800px, with an edge toggle bar (sitting on the hidden side) that swaps between panels while keeping both mounted to preserve state. Adds a useMediaQuery hook and shared split breakpoint helpers, scopes the column max-widths to desktop, and animates the panel transition.
Screen.Recording.2026-06-25.at.1.02.31.AM.mov
Summary by CodeRabbit
New Features
Bug Fixes