-
Notifications
You must be signed in to change notification settings - Fork 1.7k
style(ui): add cursor-pointer to dismiss notification button #4868
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 stories - Add withPostMessageMock decorator to mock VSCode postMessage API in Storybook - Create stories for KilocodeNotifications component demonstrating various notification states - Register decorator in Storybook preview configuration - Update extension state mock with default model property for stories
|
Code Review SummaryStatus: 2 Issues Found | Recommendation: Minor suggestions, safe to merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (5 files)
Summary: This PR adds a |
| } | ||
|
|
||
| const timers: NodeJS.Timeout[] = [] | ||
| messages.forEach((message, index) => { |
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.
SUGGESTION: Unused variable index
The index parameter is declared but never used. Consider removing it or prefixing with underscore to indicate it's intentionally unused.
| messages.forEach((message, index) => { | |
| messages.forEach((message) => { |
| }, | ||
| }, | ||
| args: { | ||
| onDismiss: fn(), |
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.
SUGGESTION: Unnecessary onDismiss prop
The KilocodeNotifications component is defined as React.FC with no props, so this onDismiss arg will be ignored. Consider removing it to avoid confusion.
| onDismiss: fn(), |
- Remove unused index parameter in withPostMessageMock forEach loop - Remove onDismiss arg from KilocodeNotifications stories since component doesn't accept it
A one line change
(and a bunch of story stuff)