-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-5573] refactor: app rail enhancements #8239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an app-rail visibility system: new types, context, and provider persisting per-workspace collapsed state; CE/EE re-exports and a CE wrapper provider; layout and content integrate the provider and conditionally render/toggle the app rail; assorted UI and icon/translation tweaks. Changes
sequenceDiagram
participant User
participant UserMenu as User Menu
participant AppRailRoot as AppRailRoot
participant Hook as useAppRailVisibility
participant Provider as AppRailVisibilityProvider
participant Storage as localStorage
participant Content as WorkspaceContentWrapper
User->>UserMenu: Click toggle item
UserMenu->>Hook: toggleAppRail()
Hook->>Provider: update isCollapsed
Provider->>Storage: persist per-workspace collapsed state
Provider->>Hook: update shouldRenderAppRail
Hook->>Content: re-render with new shouldRenderAppRail
alt shouldRenderAppRail true
Content->>AppRailRoot: render rail (add padding-left)
else shouldRenderAppRail false
Content->>AppRailRoot: skip render (remove padding-left)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces an app rail visibility management system that enables conditional rendering of the app rail navigation component based on workspace-level configuration and user preferences. It implements a clean separation between Community Edition (CE) and Enterprise Edition (EE) using the repository's established architectural pattern.
Key changes:
- Implements a React Context-based visibility system with localStorage persistence for app rail collapse state
- Establishes CE/EE separation where CE has the feature disabled by default and EE can override with custom logic
- Integrates toggle functionality into the user menu and conditionally renders the app rail in the workspace content wrapper
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
apps/web/core/lib/app-rail/types.ts |
Defines TypeScript interfaces for app rail visibility context including enabled state, collapse state, and toggle functionality |
apps/web/core/lib/app-rail/context.tsx |
Creates React Context and custom hook for consuming app rail visibility state with proper error handling |
apps/web/core/lib/app-rail/provider.tsx |
Implements the core provider component that manages app rail state using localStorage and workspace scope |
apps/web/core/lib/app-rail/index.ts |
Barrel export for the app rail library components |
apps/web/ce/hooks/app-rail/provider.tsx |
CE-specific wrapper that hardcodes isEnabled={false} to disable the app rail feature in Community Edition |
apps/web/ce/hooks/app-rail/index.ts |
Barrel export for CE app rail hooks |
apps/web/ee/hooks/app-rail/index.ts |
EE re-export pattern that delegates to CE implementation, allowing EE-specific overrides in the build |
apps/web/ce/components/workspace/content-wrapper.tsx |
Conditionally renders AppRailRoot based on visibility context and adjusts content padding accordingly |
apps/web/app/(all)/[workspaceSlug]/layout.tsx |
Wraps workspace layout with AppRailVisibilityProvider to provide context to child components |
apps/web/core/components/workspace/sidebar/user-menu-root.tsx |
Adds toggle menu item to user menu for showing/hiding the app rail when feature is enabled |
apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx |
Adds rounded corners to workspace logo border styling for visual consistency |
apps/web/core/components/workspace/sidebar/help-section/root.tsx |
Increases help icon size from 4 to 5 for better visual balance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/ce/hooks/app-rail/provider.tsx (1)
15-17: Unnecessaryobserverwrapper.This component doesn't access any MobX observables directly—it only passes
childrenthrough with a hardcodedisEnabled={false}. TheCoreProvideris already wrapped withobserver, so wrapping this thin wrapper is redundant and adds unnecessary overhead.-export const AppRailVisibilityProvider = observer(({ children }: AppRailVisibilityProviderProps) => ( +export const AppRailVisibilityProvider = ({ children }: AppRailVisibilityProviderProps) => ( <CoreProvider isEnabled={false}>{children}</CoreProvider> -)); +);If you remove
observer, you can also remove themobx-reactimport on line 4.apps/web/core/lib/app-rail/provider.tsx (1)
28-30: Stale closure intoggleAppRailcallback.The callback references
isCollapseddirectly, which captures the value at the time of memoization. If the user toggles rapidly or if there's any async behavior, this could lead to stale state.Use a functional update pattern to ensure you're always working with the latest value:
const toggleAppRail = useCallback(() => { - setIsCollapsed(!isCollapsed); -}, [isCollapsed, setIsCollapsed]); + setIsCollapsed((prev) => !prev); +}, [setIsCollapsed]);This removes the dependency on
isCollapsedand guarantees the toggle always flips the current state correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/app/(all)/[workspaceSlug]/layout.tsx(1 hunks)apps/web/ce/components/workspace/content-wrapper.tsx(1 hunks)apps/web/ce/hooks/app-rail/index.ts(1 hunks)apps/web/ce/hooks/app-rail/provider.tsx(1 hunks)apps/web/core/components/workspace/sidebar/help-section/root.tsx(1 hunks)apps/web/core/components/workspace/sidebar/user-menu-root.tsx(4 hunks)apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx(1 hunks)apps/web/core/lib/app-rail/context.tsx(1 hunks)apps/web/core/lib/app-rail/index.ts(1 hunks)apps/web/core/lib/app-rail/provider.tsx(1 hunks)apps/web/core/lib/app-rail/types.ts(1 hunks)apps/web/ee/hooks/app-rail/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/lib/app-rail/types.tsapps/web/core/components/workspace/sidebar/help-section/root.tsxapps/web/ce/hooks/app-rail/provider.tsxapps/web/core/lib/app-rail/context.tsxapps/web/core/lib/app-rail/provider.tsxapps/web/ce/hooks/app-rail/index.tsapps/web/app/(all)/[workspaceSlug]/layout.tsxapps/web/ce/components/workspace/content-wrapper.tsxapps/web/core/components/workspace/sidebar/user-menu-root.tsxapps/web/ee/hooks/app-rail/index.tsapps/web/core/lib/app-rail/index.tsapps/web/core/components/workspace/sidebar/workspace-menu-root.tsx
🧠 Learnings (5)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/lib/app-rail/context.tsxapps/web/ce/components/workspace/content-wrapper.tsx
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/core/lib/app-rail/context.tsxapps/web/core/lib/app-rail/provider.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/core/lib/app-rail/context.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/lib/app-rail/context.tsxapps/web/core/lib/app-rail/provider.tsxapps/web/app/(all)/[workspaceSlug]/layout.tsxapps/web/ce/components/workspace/content-wrapper.tsxapps/web/core/components/workspace/sidebar/user-menu-root.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `import type { Type } from "mod" with { "resolution-mode": "import" }` for specific module resolution contexts (TypeScript 5.3+)
Applied to files:
apps/web/core/lib/app-rail/index.ts
🧬 Code graph analysis (5)
apps/web/ce/hooks/app-rail/provider.tsx (1)
apps/web/core/lib/app-rail/provider.tsx (1)
AppRailVisibilityProvider(19-46)
apps/web/core/lib/app-rail/context.tsx (1)
apps/web/core/lib/app-rail/types.ts (1)
IAppRailVisibilityContext(5-26)
apps/web/app/(all)/[workspaceSlug]/layout.tsx (5)
apps/web/core/lib/wrappers/authentication-wrapper.tsx (1)
AuthenticationWrapper(26-138)apps/web/core/layouts/auth-layout/workspace-wrapper.tsx (1)
WorkspaceAuthWrapper(42-230)apps/web/ce/hooks/app-rail/provider.tsx (1)
AppRailVisibilityProvider(15-17)apps/web/core/lib/app-rail/provider.tsx (1)
AppRailVisibilityProvider(19-46)apps/web/ce/components/workspace/content-wrapper.tsx (1)
WorkspaceContentWrapper(10-37)
apps/web/ce/components/workspace/content-wrapper.tsx (2)
apps/web/core/lib/app-rail/context.tsx (1)
useAppRailVisibility(19-25)apps/web/core/components/navigation/app-rail-root.tsx (1)
AppRailRoot(16-78)
apps/web/core/components/workspace/sidebar/user-menu-root.tsx (1)
apps/web/core/lib/app-rail/context.tsx (1)
useAppRailVisibility(19-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (11)
apps/web/core/components/workspace/sidebar/help-section/root.tsx (1)
35-35: Help icon size change is safe and consistentThe bump to
size-5is a straightforward visual adjustment with no impact on behavior; looks good as part of the sidebar button item.apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx (1)
118-122: Workspace logo styling tweak looks consistentThe updated
WorkspaceLogoclassNames (border + rounded + size-7) align with the rest of the sidebar/top-nav styling; nothing functionally risky here.apps/web/core/lib/app-rail/types.ts (1)
1-27: App rail visibility context type is well-defined
IAppRailVisibilityContextmatches the provider value shape (enabled + collapsed + computed visibility + toggle) and is clearly documented; good foundation for the new context.apps/web/ee/hooks/app-rail/index.ts (1)
1-1: EE barrel re-export for app rail is straightforwardRe-exporting from
"ce/hooks/app-rail"cleanly surfaces the CE app-rail hooks under the EE path; no issues as long as thecealias is correctly configured (which matches existing patterns in this repo).apps/web/ce/hooks/app-rail/index.ts (1)
1-1: CE app rail hook barrel is clean
export * from "./provider";is a minimal, clear barrel layer that matches how other hook modules are exposed.apps/web/app/(all)/[workspaceSlug]/layout.tsx (1)
4-16: Placing AppRailVisibilityProvider at workspace layout level is appropriateWrapping
WorkspaceContentWrapper(and thus the entire workspace UI, including sidebar and content) withAppRailVisibilityProvideris the right integration point for a per‑workspace visibility context and matches how other global wrappers are structured here.apps/web/core/lib/app-rail/index.ts (1)
1-3: App rail core barrel cleanly exposes the public APIRe-exporting
./context,./provider, and./typesfrom a single index keeps consumers’ import paths simple and matches existing module organization.apps/web/ce/components/workspace/content-wrapper.tsx (1)
6-7: App rail visibility integration into workspace layout looks correct
useAppRailVisibilityis used to readshouldRenderAppRail, and<AppRailRoot />is rendered only whenshouldRenderAppRailis true, while the main content’spl-2padding is also toggled accordingly.This cleanly wires the new visibility context into the layout without changing the component’s external API.
One thing to confirm: every usage of
WorkspaceContentWrappershould now be within anAppRailVisibilityProvider(as in[workspaceSlug]/layout.tsx), otherwise the hook will throw at runtime.Also applies to: 15-17, 22-29
apps/web/core/components/workspace/sidebar/user-menu-root.tsx (1)
6-7: Localize label strings in the app rail toggle menu itemThe app rail toggle functionality is correctly wired and the provider is properly placed in the workspace layout, covering all usage of
UserMenuRoot. However, the menu item labels are hard-coded:<span>{isCollapsed ? "Show app rail" : "Hide app rail"}</span>These strings should be moved to the i18n catalog for consistency with the rest of the menu, which uses
t(...). Update the strings to:<span>{isCollapsed ? t("app_rail.show") : t("app_rail.hide")}</span>Then add the corresponding keys to the i18n catalog.
Also applies to: 17-18, 30-31, 89-100
⛔ Skipped due to learnings
Learnt from: lifeiscontent Repo: makeplane/plane PR: 7922 File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19 Timestamp: 2025-10-09T20:42:31.843Z Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.apps/web/core/lib/app-rail/context.tsx (1)
1-25: Well-structured context implementation.The context and hook follow React best practices:
- Proper
import typefor type-only imports (aligns withverbatimModuleSyntaxguidelines)- Correct undefined default with runtime guard in the hook
- Clear error message for debugging misuse
apps/web/core/lib/app-rail/provider.tsx (1)
19-19: Verifyobserveris necessary here.The component uses
observerfrom MobX but doesn't appear to access any MobX observables directly. If there's no intention to observe store changes in this provider, the wrapper can be removed to reduce overhead.If the intent is to support future observable access or if
useParamsreturns observable values in this codebase, please disregard. Otherwise, consider removing theobserverwrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/i18n/src/locales/en/translations.ts (1)
2702-2703: Navigation labels update looks good; consider casing consistency (optional).The revised phrases are clear and better aligned with the navigation UI. If you have copy guidelines around title vs sentence case, you may optionally adjust
"Tabbed Navigation"to match nearby labels (e.g.,"Tabbed navigation") or vice versa for consistency across this section.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/ce/components/app-rail/app-rail-hoc.tsx(1 hunks)apps/web/core/components/navigation/app-rail-root.tsx(3 hunks)apps/web/core/components/workspace/sidebar/user-menu-root.tsx(3 hunks)packages/i18n/src/locales/en/translations.ts(1 hunks)packages/propel/src/icons/sub-brand/plane-icon.tsx(1 hunks)packages/propel/src/icons/sub-brand/wiki-icon.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/core/components/workspace/sidebar/user-menu-root.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/propel/src/icons/sub-brand/plane-icon.tsxpackages/propel/src/icons/sub-brand/wiki-icon.tsxapps/web/core/components/navigation/app-rail-root.tsxapps/web/ce/components/app-rail/app-rail-hoc.tsxpackages/i18n/src/locales/en/translations.ts
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
packages/propel/src/icons/sub-brand/plane-icon.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
packages/propel/src/icons/sub-brand/plane-icon.tsxapps/web/core/components/navigation/app-rail-root.tsx
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/core/components/navigation/app-rail-root.tsx
🧬 Code graph analysis (4)
packages/propel/src/icons/sub-brand/plane-icon.tsx (1)
packages/propel/src/icons/icon-wrapper.tsx (1)
IconWrapper(11-44)
packages/propel/src/icons/sub-brand/wiki-icon.tsx (1)
packages/propel/src/icons/icon-wrapper.tsx (1)
IconWrapper(11-44)
apps/web/core/components/navigation/app-rail-root.tsx (1)
apps/web/core/lib/app-rail/context.tsx (1)
useAppRailVisibility(19-25)
apps/web/ce/components/app-rail/app-rail-hoc.tsx (1)
packages/propel/src/icons/sub-brand/plane-icon.tsx (1)
PlaneNewIcon(6-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/propel/src/icons/sub-brand/plane-icon.tsx (1)
8-17: LGTM! Good simplification with visual updates.The removal of
clipPathIdsimplifies the component—IconWrapper now renders the SVG paths directly without the clipPath wrapper. The updated path data changes the icon's appearance, which appears intentional for the app rail enhancements.Ensure the new icon visuals render correctly in the app rail context and match the design specifications.
packages/propel/src/icons/sub-brand/wiki-icon.tsx (1)
8-13: LGTM! Consistent refactoring pattern.This follows the same simplification as PlaneNewIcon—removing the unnecessary
clipPathIdand updating the SVG path data for the new visual design. The changes are consistent and correct.apps/web/ce/components/app-rail/app-rail-hoc.tsx (1)
20-27: Projects dock icon size change looks fineThe switch to
className="size-5"onPlaneNewIconis safe and keeps the HOC typings intact. As long as this size matches your design for other app-rail icons, there’s nothing blocking here.apps/web/core/components/navigation/app-rail-root.tsx (1)
11-12: App-rail visibility hook and dock/undock menu wiring look correct; ensure provider wrappingImporting
useAppRailVisibility, destructuring{ isCollapsed, toggleAppRail }, and wiringtoggleAppRailinto the new context-menu item is a clean integration. The conditional label ("Dock App Rail"vs"Undock App Rail") reads well and reflects the intended action given theisCollapsedflag.The only thing to double‑check is that every render path for
AppRailRootis wrapped inAppRailVisibilityProvider; otherwiseuseAppRailVisibilitywill throw if the context is missing. UsinguseParamsin this client component is consistent with the repo’s pattern of reserving routeparamsprops forpage.tsx/layout.tsxonly. Based on learnings.Also applies to: 23-24, 75-78
…p-rail-enhancements
* chore: app rail context added * chore: dock/undock app rail implementation * chore: refactor * chore: code refactor * chore: code refactor
* chore: app rail context added * chore: dock/undock app rail implementation * chore: refactor * chore: code refactor * chore: code refactor
* chore: app rail context added * chore: dock/undock app rail implementation * chore: refactor * chore: code refactor * chore: code refactor
Description
This PR includes enhancements for app rail.
Type of Change
Summary by CodeRabbit
New Features
Behavior
Style
Localization
✏️ Tip: You can customize this high-level summary in your review settings.