feat: enhance notification sidebar header with back navigation and st… #8412
Conversation
…yling improvements
WalkthroughA back navigation button using ChevronLeftIcon within a Next.js Link component is added alongside the existing Breadcrumbs header. The header now features a horizontal container layout with the new button and updated styling via getButtonStyling utilities, while maintaining existing conditional rendering logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ 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
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/components/workspace-notifications/sidebar/header/root.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Sidebar menu items should use transparent backgrounds with hover states like `hover:bg-layer-1-hover` rather than base layer backgrounds, maintaining visual lightness while providing hover feedback
Applied to files:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
📚 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/components/workspace-notifications/sidebar/header/root.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/components/workspace-notifications/sidebar/header/root.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/plane/ui/**/*.{ts,tsx} : Build components in `plane/ui` package with Storybook for isolated development
Applied to files:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
📚 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:
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/core/components/workspace-notifications/sidebar/header/root.tsx (3)
2-8: LGTM! Clean import organization.The new imports are properly structured and necessary for the navigation and styling enhancements.
28-28: LGTM! Appropriate layout structure.The flex container with gap spacing properly organizes the back button and breadcrumbs horizontally.
38-48: LGTM! Breadcrumbs properly configured.The Breadcrumbs implementation correctly uses translations, icon sizing, and the BreadcrumbLink component pattern.
| <Link | ||
| href={`/${workspaceSlug}`} | ||
| className={cn( | ||
| getButtonStyling("secondary", "base"), | ||
| "flex items-center justify-center gap-2 text-tertiary rounded-lg h-6 w-6 p-1 hover:bg-surface-2 hover:text-secondary" | ||
| )} | ||
| > | ||
| <ChevronLeftIcon className="h-4 w-4" /> | ||
| </Link> |
There was a problem hiding this comment.
Add accessible label for screen reader users.
The back navigation button contains only a visual icon without any accessible text or aria-label. Screen readers cannot announce the button's purpose, violating WCAG 2.1 Level A (Success Criterion 2.4.4 - Link Purpose).
🔎 Proposed fix to add aria-label
<Link
href={`/${workspaceSlug}`}
+ aria-label={t("notification.back_to_workspace")}
className={cn(
getButtonStyling("secondary", "base"),
"flex items-center justify-center gap-2 text-tertiary rounded-lg h-6 w-6 p-1 hover:bg-surface-2 hover:text-secondary"
)}
>
<ChevronLeftIcon className="h-4 w-4" />
</Link>Note: You'll need to add the translation key "notification.back_to_workspace" to the i18n files, or use an existing appropriate translation.
Consider simplifying the button styling approach.
The implementation combines getButtonStyling("secondary", "base") with extensive custom className overrides (rounded-lg, h-6, w-6, p-1, text colors, hover states). This pattern suggests either:
- The custom classes are unnecessary and conflict with getButtonStyling output
- Or getButtonStyling is not the right abstraction for this use case
Consider using only custom classes or creating a dedicated icon-button variant in the design system.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/core/components/workspace-notifications/sidebar/header/root.tsx
around lines 29-37, the Link back button only contains an icon and lacks an
accessible label; add an aria-label prop that uses the i18n key
"notification.back_to_workspace" (or an existing appropriate translation) so
screen readers can announce the link, and ensure the translation key is added to
locale files. Also simplify styling: either remove getButtonStyling and keep the
explicit icon-button classes (rounded-lg, h-6, w-6, p-1, text/hover classes) or
replace the combined classes with a dedicated "icon-button" variant in
getButtonStyling to avoid class conflicts; pick one approach and apply it
consistently to this component.
|
I wasn't able to run the prettier fix. Let me know if this can go through or needs any change. |
1 participant
Description
Added a Back to workspace button to the notifications sidebar header so users can easily navigate from /[workspaceSlug]/notifications back to the workspace home.
The back button:
Type of Change
Test Scenarios
/${workspaceSlug}.apps/web/core/components/workspace-notifications/sidebar/header/root.tsx