feat: consolodate header bars#496
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThis PR refactors the File Tree header UI by removing the standalone FileTreeHeader component and consolidating tab and panel management. A new WorkspacePanelHeader component replaces the file tree header functionality, allowing the tab bar to be hidden when only the File Tree tab exists. The create-directory button is relocated from the file tree header to the workspace panel action bars. Supporting changes include tab styling updates for the new layout, panel shell and header CSS refinements, conditional rendering of tab close buttons based on tab type, and a React element validation fix in the Tabs wrapper component. 🚥 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/components/project-editor/project-editor.tsx (1)
465-465: 💤 Low valueConsider removing the
as anytype assertion.The type assertion bypasses TypeScript's type checking. If
tabListStylehas a type incompatibility with emotion'scssprop, consider fixing the type definition intab-styles.tsxinstead.🤖 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 `@src/components/project-editor/project-editor.tsx` at line 465, Remove the unsafe "as any" on the css prop and fix the style's type in tab-styles.tsx so it matches Emotion's expected types; specifically update the tabListStyle export in tab-styles.tsx to be a SerializedStyles or an Interpolation<Theme> (importing the correct types from "`@emotion/react`"), then remove "as any" in project-editor.tsx where css={tabListStyle as any} so the css prop is passed a correctly typed tabListStyle.src/components/project-editor/styles.ts (1)
223-223: 💤 Low valueConsider if
margin-left: autois necessary.Given that
panelActionGroupalready usesjustify-content: flex-endwithin a flex container,margin-left: automay be redundant depending on the parent layout.Verify whether this is needed or if the parent's
justify-content: space-between(inpanelTopBar) already handles the positioning.🤖 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 `@src/components/project-editor/styles.ts` at line 223, The CSS rule setting margin-left: auto on panelActionGroup may be redundant because panelActionGroup is inside a flex container that already uses justify-content: flex-end (and parent panelTopBar may use justify-content: space-between); review the layout and remove margin-left: auto from the panelActionGroup style if positioning is already handled by the parent flex rules, or if removal breaks alignment adjust panelTopBar/panelActionGroup flex settings (justify-content or flex-grow) so placement is achieved without margin-left: auto.src/components/project-editor/tab-styles.tsx (2)
119-119: ⚡ Quick winHardcoded colors may break theme consistency.
The active and hover backgrounds use hardcoded
rgba(0, 0, 0, ...)values instead of theme colors. This could cause visibility issues with light themes or when users switch themes, as the dark overlay may not provide sufficient contrast.Consider using theme-based colors for consistency:
♻️ Suggested theme-based colors
background-color: ${(properties: any) => - properties.active ? "rgba(0, 0, 0, 0.16)" : "transparent"}; + properties.active ? properties.theme.highlightBackgroundAlt : "transparent"}; border: 0;background-color: ${(properties: any) => properties.active - ? "rgba(0, 0, 0, 0.2)" + ? properties.theme.highlightBackground : properties.theme.highlightBackground};Also applies to: 134-135
🤖 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 `@src/components/project-editor/tab-styles.tsx` at line 119, The hardcoded rgba background values (e.g., the expression using properties.active and the hover backgrounds at the nearby lines) break theme consistency; replace those hardcoded "rgba(0, 0, 0, ...)" values with theme-aware tokens by reading from the component theme (e.g., use theme.palette.action.hover, theme.palette.action.selected, or generate an alpha of theme.palette.text.primary via theme.fn.alpha or an equivalent utility) so the active and hover backgrounds follow light/dark themes—update the styled component(s) that reference properties.active and the hover rules to use the theme calls instead of literal rgba strings.
48-48: ⚡ Quick winConsider removing
!importantby addressing specificity.The
!importantdeclaration suggests a specificity conflict with other styles. This can make the stylesheet harder to maintain and debug.Consider refactoring to increase selector specificity or reorder CSS rules to avoid needing
!important.🤖 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 `@src/components/project-editor/tab-styles.tsx` at line 48, Remove the "!important" on the background-color rule in src/components/project-editor/tab-styles.tsx and resolve the specificity conflict instead: either make the selector more specific (e.g., target the exact class/element used for the tab, or add a component-specific class) or move this rule later in the stylesheet so it overrides the conflicting rule; alternatively apply the background via a React style prop or the component's className to ensure the rule wins without using !important (search for the background-color: transparent !important; line to locate the exact selector to change).
🤖 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.
Nitpick comments:
In `@src/components/project-editor/project-editor.tsx`:
- Line 465: Remove the unsafe "as any" on the css prop and fix the style's type
in tab-styles.tsx so it matches Emotion's expected types; specifically update
the tabListStyle export in tab-styles.tsx to be a SerializedStyles or an
Interpolation<Theme> (importing the correct types from "`@emotion/react`"), then
remove "as any" in project-editor.tsx where css={tabListStyle as any} so the css
prop is passed a correctly typed tabListStyle.
In `@src/components/project-editor/styles.ts`:
- Line 223: The CSS rule setting margin-left: auto on panelActionGroup may be
redundant because panelActionGroup is inside a flex container that already uses
justify-content: flex-end (and parent panelTopBar may use justify-content:
space-between); review the layout and remove margin-left: auto from the
panelActionGroup style if positioning is already handled by the parent flex
rules, or if removal breaks alignment adjust panelTopBar/panelActionGroup flex
settings (justify-content or flex-grow) so placement is achieved without
margin-left: auto.
In `@src/components/project-editor/tab-styles.tsx`:
- Line 119: The hardcoded rgba background values (e.g., the expression using
properties.active and the hover backgrounds at the nearby lines) break theme
consistency; replace those hardcoded "rgba(0, 0, 0, ...)" values with
theme-aware tokens by reading from the component theme (e.g., use
theme.palette.action.hover, theme.palette.action.selected, or generate an alpha
of theme.palette.text.primary via theme.fn.alpha or an equivalent utility) so
the active and hover backgrounds follow light/dark themes—update the styled
component(s) that reference properties.active and the hover rules to use the
theme calls instead of literal rgba strings.
- Line 48: Remove the "!important" on the background-color rule in
src/components/project-editor/tab-styles.tsx and resolve the specificity
conflict instead: either make the selector more specific (e.g., target the exact
class/element used for the tab, or add a component-specific class) or move this
rule later in the stylesheet so it overrides the conflicting rule; alternatively
apply the background via a React style prop or the component's className to
ensure the rule wins without using !important (search for the background-color:
transparent !important; line to locate the exact selector to change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 479312b3-ba07-47e1-8b71-3c1bc96f0df6
📒 Files selected for processing (7)
src/components/file-tree/header.tsxsrc/components/file-tree/index.tsxsrc/components/project-editor/project-editor.tsxsrc/components/project-editor/styles.tssrc/components/project-editor/tab-styles.tsxsrc/tabtab/tab.jsxsrc/tabtab/tabs.jsx
💤 Files with no reviewable changes (2)
- src/components/file-tree/header.tsx
- src/components/file-tree/index.tsx
1 participant
currently there's redundancy where have 2 and 3 header bars, which we can compress together