Feat/update announcement #1824
Feat/update announcement #1824
Conversation
- Add custom megaphone icon using Lucide v0.15.31 SVG design - Implement unread state management with conditional dot display - Position button left of Run button in V2Header - Add hover effect with opacity transition - Include accessibility support with proper aria-label and title
- Add Dialog components from @giselle-internal/ui - Implement full dialog with header, content, and footer sections - Add 3 update items with NEW/IMPROVED labels and descriptions - Use consistent color palette (#DAFF09, #08F4EE, #1663F3) - Add hover effects and chevron navigation icons - Optimize padding by using negative margin (-m-3) approach - Remove unnecessary border lines and margins for cleaner look - Include proper accessibility attributes and keyboard navigation
- Add individual dates (Dec 15, 12, 10, 2024) to each update item - Replace 'Got it!' button with 'View all' link directing to blog updates - Improve code formatting consistency (tabs to spaces) - Enhance content structure for better readability - Maintain accessibility and hover effects
- Add UpdateNotificationButton to studio.giselles.ai main layout header - Export UpdateNotificationButton through proper module export chain - Position notification icon between Docs link and UserButton - Maintain consistent UI spacing and styling in header - Follow 'less is more' philosophy with minimal, obvious implementation
- Move UpdateNotificationButton to the left of Docs link in header navigation - Maintain consistent spacing and visual hierarchy - Follow user's requested positioning preference
- Add UpdateNotificationProvider with React Context for global state - Implement safe hook that works with/without provider (graceful fallback) - Sync notification state across all instances (Studio, Playground, Editor) - One-click update to dialog content reflects everywhere - Maintain existing UI/UX while eliminating duplicate state management ✅ Single source of truth - update once, reflects everywhere ✅ Graceful degradation when provider missing ✅ Maintains all existing functionality ✅ Zero breaking changes to existing code
…vider cleanup - Extract notification data to structured constants for easier maintenance - Remove complex fallback logic in favor of clear provider requirement - Eliminate redundant UpdateNotificationProvider in V2Placeholder - Generate notification items dynamically with .map() for better scalability - Reduce code complexity while maintaining full functionality - Improve readability and maintainability without breaking changes
- Add UpdateNotificationButton integration to navigation items - Add notification type support to NavigationListItem - Make entire Updates navigation item clickable (both icon and text) - Maintain consistent styling with other navigation items - Show notification dot when unread updates exist
|
|
Finished running flow.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. WalkthroughIntroduces an Update Notification feature: new notification-type nav item, a context provider, and a button that opens a dialog listing hard-coded updates. Integrates the button into the editor header and navigation rail. Adds re-exports to surface provider/button in the workflow-designer-ui package. Minor import cleanup elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant NR as NavigationListItem (notification)
participant UNB as UpdateNotificationButton
participant C as UpdateNotificationProvider (Context)
participant D as Dialog
U->>NR: Click "Updates" nav item
NR->>UNB: Forward click to inner button
UNB->>C: set isOpen = true, markAsRead()
C-->>UNB: Context state updated (hasUnread=false)
UNB->>D: Render Dialog (open)
U->>D: Interact with list / View all
D-->>UNB: Close request
UNB->>C: set isOpen = false
sequenceDiagram
autonumber
actor U as User
participant VH as V2Header
participant UNB as UpdateNotificationButton
participant C as UpdateNotificationProvider
participant D as Dialog
U->>VH: Click bell/megaphone
VH->>UNB: Event
UNB->>C: Toggle isOpen, markAsRead()
UNB->>D: Show Latest Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
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 |
📝 E2E Test Generation Prompt4. MCP Integration Guidelines (Optional)Playwright tests are executed via a command-line interface. If a proprietary "Playwright MCP" tool is used, it likely wraps the standard Playwright CLI.
5. CI-Ready Code Requirements
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds an Updates notification button to the stage navigation rail with consistent styling and clickable functionality. The implementation introduces a notification system that shows a blue dot when unread updates exist and opens a dialog with the latest feature announcements.
- Add UpdateNotificationButton component with notification management context
- Integrate notifications into stage navigation rail and main layout header
- Update navigation system to support notification-type items alongside existing link items
Reviewed Changes
Copilot reviewed 11 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx | New component implementing notification button with dialog and state management |
| apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx | Extended to support notification type navigation items |
| apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-items.ts | Added Updates notification item to navigation configuration |
| internal-packages/workflow-designer-ui/src/editor/v2/components/v2-header.tsx | Integrated notification button into editor header |
| apps/studio.giselles.ai/app/(main)/layout.tsx | Added notification button to main layout header |
| README.md | Major restructuring with enhanced styling and feature showcase |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| useUpdateNotification(); | ||
|
|
||
| const handleClick = () => { | ||
| console.log("Update notification clicked!"); |
There was a problem hiding this comment.
Remove console.log statements from production code. These debug statements should be removed or replaced with proper logging if needed.
| console.log("Update notification clicked!"); |
| onClick={() => | ||
| console.log(`Navigate to ${notification.title} details`) | ||
| } |
There was a problem hiding this comment.
Remove console.log statements from production code. These debug statements should be removed or replaced with proper logging if needed.
| onClick={() => | |
| console.log(`Navigate to ${notification.title} details`) | |
| } | |
| onClick={() => {}} |
| type: "notification", | ||
| icon: MegaphoneIcon, | ||
| label: "Updates", | ||
| onClick: () => {}, |
There was a problem hiding this comment.
The onClick handler is empty. This should either implement the intended functionality or be removed if the notification behavior is handled elsewhere.
| onClick: () => {}, | |
| onClick: () => { console.log("Updates notification clicked"); }, |
| onClick={(e) => { | ||
| const updateButton = e.currentTarget.querySelector("button"); | ||
| if (updateButton) { | ||
| updateButton.click(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Using querySelector to find and programmatically click a button creates fragile coupling. Consider passing the click handler directly or using a more explicit event delegation pattern.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
- Changed from switch statement to if-else chain for better type narrowing - TypeScript now properly narrows the NavigationItem union type - Fixes Vercel deployment build error 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![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
♻️ Duplicate comments (2)
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx (1)
113-116: Remove console.log in production pathsDrop debug logs in the trigger and item clicks; wire real navigation later.
- console.log("Update notification clicked!"); markAsRead();- onClick={() => - console.log(`Navigate to ${notification.title} details`) - } + onClick={() => { + /* TODO: navigate to notification details when available */ + }}Also applies to: 146-152
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx (1)
23-40: Critical: Nested button + querySelector click-forwarding breaks HTML/a11yA button inside a button is invalid and harms accessibility. Programmatic forwarding via querySelector is brittle. Render the update trigger as the row itself.
- if (props.type === "notification") { - return ( - <button - type="button" - onClick={(e) => { - const updateButton = e.currentTarget.querySelector("button"); - if (updateButton) { - updateButton.click(); - } - }} - className="text-text-muted text-sm flex items-center py-0.5 hover:bg-ghost-element-hover rounded-lg px-1 w-full text-left" - > - <div className="size-8 flex items-center justify-center"> - <UpdateNotificationButton /> - </div> - {props.variant === "expanded" && props.label} - </button> - ); - } + if (props.type === "notification") { + return ( + <UpdateNotificationButton variant="nav-item"> + {props.variant === "expanded" && props.label} + </UpdateNotificationButton> + ); + }
🧹 Nitpick comments (4)
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx (4)
111-111: Ensure unread badge clears on any open pathWhen the dialog opens via other triggers, also clear unread to prevent stale dots.
- const { hasUnreadUpdates, isOpen, setIsOpen, markAsRead } = context; + const { + hasUnreadUpdates, + isOpen, + setIsOpen, + setHasUnreadUpdates, + markAsRead, + } = context; @@ - <Dialog open={isOpen} onOpenChange={setIsOpen}> + <Dialog + open={isOpen} + onOpenChange={(open) => { + setIsOpen(open); + if (open) setHasUnreadUpdates(false); + }} + >Also applies to: 119-121
72-101: Export a small hook for context access (future-proofing consumers)A
useUpdateNotification()hook lets other components open the dialog without DOM querying.const UpdateNotificationContext = createContext<UpdateNotificationState | null>( null, ); export function UpdateNotificationProvider({ @@ } +export function useUpdateNotification() { + const ctx = useContext(UpdateNotificationContext); + if (!ctx) { + throw new Error( + "useUpdateNotification must be used within UpdateNotificationProvider", + ); + } + return ctx; +}
130-130: Prefer design tokens over hard-coded hexUse a theme token/utility (e.g., accent/brand color) for the dot to keep dark/light modes consistent.
41-61: Consider using lucide-react’s Megaphone to reduce custom SVGKeeps iconography consistent and reduces maintenance surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx(1 hunks)internal-packages/workflow-designer-ui/src/editor/v2/components/index.ts(1 hunks)internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx(1 hunks)packages/giselle/src/react/act/use-act-controller.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- packages/giselle/src/react/act/use-act-controller.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- internal-packages/workflow-designer-ui/src/editor/v2/components/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-guide.mdc)
**/*.{ts,tsx}: Use Biome for formatting with tab indentation and double quotes
Follow organized imports pattern (enabled in biome.json)
Use TypeScript for type safety; avoidanytypes
Use Next.js patterns for web applications
Use async/await for asynchronous code rather than promises
Error handling: use try/catch blocks and propagate errors appropriately
Use kebab-case for all filenames (e.g.,user-profile.ts)
Use camelCase for variables, functions, and methods (e.g.,userEmail)
Use prefixes likeis,has,can,shouldfor boolean variables and functions for clarity
Use verbs or verb phrases that clearly indicate purpose for function naming (e.g.,calculateTotalPrice(), notprocess())If breaking changes are introduced in new AI SDK versions, update code to accommodate those changes
**/*.{ts,tsx}: Use TypeScript and avoid any
Use async/await and proper error handling
Variables and functions should use camelCase
Boolean variables and functions should use is/has/can/should prefixes
Function names should clearly indicate purpose
Files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsxinternal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development-guide.mdc)
**/*.tsx: Use functional components with React hooks
Use PascalCase for React components and classes (e.g.,UserProfile)
**/*.tsx: Components should use React hooks and Next.js patterns
Component identifiers should use PascalCase
Files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsxinternal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-guide.mdc)
All filenames should use kebab-case (lowercase with hyphens)
Files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsxinternal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-guide.mdc)
**/*.{js,jsx,ts,tsx}: React components and classes should use PascalCase
Variables, functions, and methods should use camelCase
Use verbs or verb phrases for function names; names should clearly indicate what the function does; avoid ambiguous names that could lead to misuse
Use nouns or noun phrases for variable names; names should describe what the variable represents; avoid single-letter variables except in very short scopes
Use prefixes like 'is', 'has', 'can', 'should' for both variables and functions returning boolean values; make the true/false meaning clear
Files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsxinternal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-list*.tsx
📄 CodeRabbit inference engine (apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md)
apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-list*.tsx: If you need non-link behaviors, extend NavigationListItem to support additional item types
Compute the current pathname in NavigationListItem and apply active styles when navigationItem.isActive?.(pathname) is true
Alternatively, pass the active pathname down from the page/layout and style items in NavigationListItem accordingly
Files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: giselles-ai/giselle#0
File: internal-packages/workflow-designer-ui/src/new-editor/AGENTS.md:0-0
Timestamp: 2025-09-02T05:50:06.317Z
Learning: Applies to internal-packages/workflow-designer-ui/src/new-editor/**/*.ts : Lift actions into the store (e.g., expose updateNode) and invoke them from components performing mutations.
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-list*.tsx : If you need non-link behaviors, extend NavigationListItem to support additional item types
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/{navigation-rail-collapsed.tsx,menu-button.tsx} : Update collapsed header toggle icons or hover behavior
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-list*.tsx : Alternatively, pass the active pathname down from the page/layout and style items in NavigationListItem accordingly
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-items.ts : To change labels, icons, or routes, update the corresponding fields in the nav item objects
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-items.ts : Define navigation destinations by appending objects shaped { id, type: "link", icon, label, href, isActive? }
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-rail.tsx : Adjust the default rail state by changing useState<NavigationRailState>("expanded") to "collapsed" (or vice versa)
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-rail.tsx : If moving away from Suspense-based user loading, change prop to user: UserDataForNavigationRail
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-list*.tsx : Compute the current pathname in NavigationListItem and apply active styles when navigationItem.isActive?.(pathname) is true
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-rail-footer-menu.tsx : Modify footer/account menu by editing the items array; set external: true for external links (renders <a target="_blank" rel="noopener">); the value: "log-out" item renders SignOutButton via renderItem
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-01T00:43:10.540Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: apps/studio.giselles.ai/app/stage/ui/navigation-rail/AGENTS.md:0-0
Timestamp: 2025-09-01T00:43:10.540Z
Learning: Applies to apps/studio.giselles.ai/app/stage/ui/navigation-rail/**/navigation-items.ts : Import icons for navigation items from lucide-react or the internal icon set when adding/updating items
Applied to files:
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx
📚 Learning: 2025-09-02T05:50:06.317Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: internal-packages/workflow-designer-ui/src/new-editor/AGENTS.md:0-0
Timestamp: 2025-09-02T05:50:06.317Z
Learning: Applies to internal-packages/workflow-designer-ui/src/new-editor/**/*.ts : Lift actions into the store (e.g., expose updateNode) and invoke them from components performing mutations.
Applied to files:
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
📚 Learning: 2025-07-21T22:29:07.662Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: .cursor/rules/edit-workspace-tour.mdc:0-0
Timestamp: 2025-07-21T22:29:07.662Z
Learning: Applies to internal-packages/workflow-designer-ui/src/editor/workspace-tour/workspace-tour.tsx : Update the `TourGlobalStyles` component in `workspace-tour.tsx` for animation changes
Applied to files:
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
📚 Learning: 2025-09-02T05:50:06.317Z
Learnt from: CR
PR: giselles-ai/giselle#0
File: internal-packages/workflow-designer-ui/src/new-editor/AGENTS.md:0-0
Timestamp: 2025-09-02T05:50:06.317Z
Learning: Applies to internal-packages/workflow-designer-ui/src/new-editor/**/*.tsx : Use selective subscriptions: selectors must pick only the minimal state needed for render (e.g., s.nodesById[id], specific UI flags).
Applied to files:
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
🧬 Code graph analysis (2)
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-list-item.tsx (4)
apps/studio.giselles.ai/app/stage/ui/navigation-rail/navigation-items.ts (1)
NavigationItem(21-21)apps/studio.giselles.ai/app/stage/ui/navigation-rail/types.ts (1)
NavigationRailState(1-1)internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx (1)
UpdateNotificationButton(104-203)internal-packages/workflow-designer-ui/src/editor/v2/components/index.ts (1)
UpdateNotificationButton(2-2)
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx (2)
internal-packages/workflow-designer-ui/src/editor/v2/components/index.ts (2)
UpdateNotificationProvider(3-3)UpdateNotificationButton(2-2)internal-packages/workflow-designer-ui/src/editor/index.tsx (2)
UpdateNotificationProvider(24-24)UpdateNotificationButton(24-24)
| export function UpdateNotificationButton() { | ||
| const context = useContext(UpdateNotificationContext); | ||
| if (!context) { | ||
| throw new Error( | ||
| "UpdateNotificationButton must be used within UpdateNotificationProvider", | ||
| ); | ||
| } | ||
| const { hasUnreadUpdates, isOpen, setIsOpen, markAsRead } = context; | ||
|
|
||
| const handleClick = () => { | ||
| console.log("Update notification clicked!"); | ||
| markAsRead(); | ||
| }; | ||
|
|
||
| return ( | ||
| <Dialog open={isOpen} onOpenChange={setIsOpen}> | ||
| <DialogTrigger asChild> | ||
| <button | ||
| type="button" | ||
| onClick={handleClick} | ||
| className="relative" | ||
| aria-label="View updates" | ||
| > | ||
| <MegaphoneIcon className="size-4 text-text-muted" /> | ||
| {/* Notification dot - only show when there are unread updates */} | ||
| {hasUnreadUpdates && ( | ||
| <div className="absolute -top-[2px] -right-[2px] w-[8px] h-[8px] bg-[#6B8FF0] rounded-full" /> | ||
| )} | ||
| </button> | ||
| </DialogTrigger> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid nested-button integrations: Add a full-width “nav-item” trigger variant
Expose a variant and children to let consumers render the entire navigation row as the trigger, eliminating the need to wrap this component in another button.
-export function UpdateNotificationButton() {
+export function UpdateNotificationButton({
+ variant = "icon",
+ className,
+ children,
+}: {
+ variant?: "icon" | "nav-item";
+ className?: string;
+ children?: ReactNode;
+}) {
@@
- const { hasUnreadUpdates, isOpen, setIsOpen, markAsRead } = context;
+ const { hasUnreadUpdates, isOpen, setIsOpen, markAsRead } = context;
@@
- <DialogTrigger asChild>
- <button
- type="button"
- onClick={handleClick}
- className="relative"
- aria-label="View updates"
- >
- <MegaphoneIcon className="size-4 text-text-muted" />
- {/* Notification dot - only show when there are unread updates */}
- {hasUnreadUpdates && (
- <div className="absolute -top-[2px] -right-[2px] w-[8px] h-[8px] bg-[#6B8FF0] rounded-full" />
- )}
- </button>
- </DialogTrigger>
+ <DialogTrigger asChild>
+ <button
+ type="button"
+ onClick={handleClick}
+ className={`${variant === "nav-item"
+ ? "text-text-muted text-sm flex items-center py-0.5 hover:bg-ghost-element-hover rounded-lg px-1 w-full text-left"
+ : "relative"} ${className ?? ""}`}
+ aria-label="View updates"
+ >
+ {variant === "nav-item" ? (
+ <>
+ <div className="size-8 flex items-center justify-center relative">
+ <MegaphoneIcon className="size-4 text-text-muted" />
+ {hasUnreadUpdates && (
+ <div className="absolute -top-[2px] -right-[2px] w-[8px] h-[8px] bg-[#6B8FF0] rounded-full" />
+ )}
+ </div>
+ {children}
+ </>
+ ) : (
+ <>
+ <MegaphoneIcon className="size-4 text-text-muted" />
+ {hasUnreadUpdates && (
+ <div className="absolute -top-[2px] -right-[2px] w-[8px] h-[8px] bg-[#6B8FF0] rounded-full" />
+ )}
+ </>
+ )}
+ </button>
+ </DialogTrigger>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
internal-packages/workflow-designer-ui/src/editor/v2/components/update-notification-button.tsx
lines 104-133: the component currently renders an internal <button> inside
DialogTrigger which forces consumers to wrap it in another button for full-width
nav rows; add a variant prop (e.g., variant?: 'icon' | 'nav-item') and children
prop, and when variant === 'nav-item' render DialogTrigger with asChild and
forward the provided children (no internal button) so consumers can supply the
full-width nav row element as the trigger; when variant === 'icon' keep the
existing internal button and attach handleClick/aria-label/notification-dot to
it; ensure types support children and variant, and that markAsRead/onClick are
only called once by wiring handleClick to the consumed child via cloning or
letting consumers call it (document in prop types).
shige
left a comment
shige
left a comment
There was a problem hiding this comment.
The following concerns me:
- Clicking on each notification element does nothing.
- The Latest Updates section contains dummy content. I'd like it to be mergeable.
|
@toyamarinyon |
|
@shige |
|
It looks good overall, but there are some areas that need some adjustment, so I will check and adjust them at the start of next week. |
| const { hasUnreadUpdates, isOpen, setIsOpen, markAsRead } = context; | ||
|
|
||
| const handleClick = () => { | ||
| console.log("Update notification clicked!"); |
There was a problem hiding this comment.
🟡 MEDIUM - Debug console.log left in production code
Category: quality
| Agent: General Code Review
Description:
console.log statement appears to be debug leftover code
Suggestion:
Remove debug console.log or use proper logger if intentional logging is needed
Confidence: 95%
Rule: general_console_log_committed
| key={notification.id} | ||
| type="button" | ||
| onClick={() => | ||
| console.log(`Navigate to ${notification.title} details`) |
There was a problem hiding this comment.
🟡 MEDIUM - Debug console.log left in production code
Category: quality
| Agent: General Code Review
Description:
console.log statement appears to be debug leftover code
Suggestion:
Remove debug console.log or use proper logger if intentional logging is needed
Confidence: 95%
Rule: general_console_log_committed
| function MegaphoneIcon({ className }: { className?: string }) { | ||
| return ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width="24" | ||
| height="24" | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| strokeWidth="2" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| className={className} | ||
| > | ||
| <title>Megaphone icon</title> | ||
| <path d="M11 6a13 13 0 0 0 8.4-2.8A1 1 0 0 1 21 4v12a1 1 0 0 1-1.6.8A13 13 0 0 0 11 14H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2z" /> | ||
| <path d="M6 14a12 12 0 0 0 2.4 7.2 2 2 0 0 0 3.2-2.4A8 8 0 0 1 10 14" /> | ||
| <path d="M8 6v8" /> | ||
| </svg> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Custom MegaphoneIcon reimplements lucide-react icon
Category: quality
| Agent: Code Consistency & Reuse Check
Description:
Creating custom SVG component when MegaphoneIcon already exists in lucide-react library
Suggestion:
Import MegaphoneIcon from 'lucide-react' instead of defining a custom component. The file already imports ChevronRightIcon from the same library (line 9), and navigation-items.ts (line 2) shows MegaphoneIcon is available from lucide-react.
Confidence: 90%
Rule: quality_reinventing_wheel
|
User description
Summary
Add Updates notification button to the stage navigation rail with consistent styling and clickable functionality.
Related Issue
N/A
Changes
Testing
Other Information
Implementation maintains existing design system and follows established patterns in the navigation rail components.
I'll update the dialog content during the merge. Hopefully the agent will be ready for the update by then...
PR Type
Enhancement
Description
Add update notification button to navigation systems
Implement global state management for notifications
Integrate notification UI across Studio and Editor
Update README with enhanced branding and features
Diagram Walkthrough
File Walkthrough
10 files
Add notification navigation item typeExport update notification componentsAdd components export to v2 moduleIntegrate notification provider and buttonAdd notification item rendering supportWrap navigation rail with providerAdd notification provider to workspaceExport notification components from editorImplement complete notification system componentAdd notification button to header1 files
Complete README redesign with enhanced brandingSummary by CodeRabbit
New Features
UI
Chores