-
Notifications
You must be signed in to change notification settings - Fork 3.2k
improvement: popover, tooltip, notifications #2676
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
…ions): loading content
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR adds several improvements to the popover and tooltip components in the EMCN library: New Features:
Improvements:
Bug Fix:
Minor Changes:
The changes follow established EMCN component patterns and improve visual consistency across context menus. However, the PR includes globals.css modifications which violate project guidelines. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ContextMenu as Context Menu Component
participant Popover as Popover Root
participant PopoverContent
participant PopoverItem
User->>ContextMenu: Right-click (block/pane/workflow)
ContextMenu->>Popover: Set open={true}, colorScheme='inverted'
Popover->>PopoverContent: Render with inverted styles
PopoverContent->>PopoverContent: Apply colorScheme colors from STYLES
Note over PopoverContent: Groups separated by PopoverDivider
PopoverContent->>PopoverItem: Render menu items
PopoverItem->>PopoverItem: Apply getItemStateClasses(variant, colorScheme)
alt User hovers item
User->>PopoverItem: Mouse enter
PopoverItem->>PopoverItem: Apply hover state from STYLES.states.inverted
end
User->>PopoverItem: Click action
PopoverItem->>ContextMenu: Execute callback (onCopy, onDelete, etc)
ContextMenu->>Popover: Set open={false}
Popover->>PopoverContent: Unmount
|
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.
Additional Comments (1)
-
apps/sim/app/_styles/globals.css, line 50-100 (link)style: violates custom instruction about avoiding globals.css edits
Custom instruction states: "Avoid editing the globals.css file unless absolutely necessary. Move style changes to local component files instead."
Consider moving the scrollbar styling changes to local component styles. The
--text-muted-inversevariable addition is reasonable for the inverted color scheme.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Context Used: Context from
dashboard- Avoid editing the globals.css file unless absolutely necessary. Move style changes to local componen... (source)
17 files reviewed, 1 comment
…ions): loading content (#2676)
…ions): loading content (#2676)
Summary
Feat(popover): sections
Improvement: tooltip, popover
Fix(notifications): loading content
Type of Change
Testing
Solo.
Checklist