-
Notifications
You must be signed in to change notification settings - Fork 1
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
BA-2205 Copy messages to clipboard #207
Conversation
|
WalkthroughThis pull request updates the Storybook configuration to include new story paths for design system providers. It also enhances the notification system by adding a new parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MI as MessageItem
participant UN as useNotification
participant SB as SnackbarProvider
participant T as Timer
U->>MI: Click "Copy" action
MI->>UN: Call sendToast(message, { shouldShowProgress })
UN->>SB: Trigger display of notification
SB->>T: Set timeout (HIDE_DURATION) for auto-close
T-->>SB: Timeout expires
SB->>SB: Close notification and clear timer
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/design-system/.eslintrc.js
packages/utils/hooks/useNotification/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/utils/.eslintrc.js
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (6)
packages/design-system/providers/SnackbarProvider/index.tsx (1)
62-79
: Styling improvements.
Your use ofanchorOrigin={{ vertical: 'bottom', horizontal: 'right' }}
and custom styling (e.g., white background, black text) helps differentiate this notification style. Consider referencing theme spacing or breakpoints (instead of fixedcalc(100% - 16px)
) if you anticipate more dynamic layouts in the future.packages/design-system/providers/SnackbarProvider/constants.tsx (1)
4-4
: Add documentation for the HIDE_DURATION constant.Consider adding a JSDoc comment explaining the purpose and unit (milliseconds) of this constant.
+/** Duration in milliseconds before the snackbar automatically hides */ export const HIDE_DURATION = 3000
packages/design-system/providers/SnackbarProvider/styled.tsx (1)
4-22
: Consider extracting magic numbers into constants.The styling values like minWidth, borderRadius, and shadow values could be extracted into theme constants for better maintainability and consistency.
+const SNACKBAR_CONSTANTS = { + MIN_WIDTH: '360px', + BORDER_RADIUS: '8px', + BOX_SHADOW: '0px 8px 16px 0px rgba(145, 158, 171, 0.16)', + MAX_WIDTH: { + DEFAULT: '60%', + MD: '80%', + SM: '100%' + } +} export const SnackbarContentContainer = styled(Box)(({ theme }) => ({ display: 'flex', - minWidth: '360px', + minWidth: SNACKBAR_CONSTANTS.MIN_WIDTH, flexDirection: 'column', justifyContent: 'center', alignItems: 'flex-start', - borderRadius: '8px', + borderRadius: SNACKBAR_CONSTANTS.BORDER_RADIUS, border: `1px solid ${theme.palette.grey[200]}`, backgroundColor: theme.palette.common.white, - boxShadow: '0px 8px 16px 0px rgba(145, 158, 171, 0.16)', + boxShadow: SNACKBAR_CONSTANTS.BOX_SHADOW, overflow: 'hidden', - maxWidth: '60%', + maxWidth: SNACKBAR_CONSTANTS.MAX_WIDTH.DEFAULT, [theme.breakpoints.down('md')]: { - maxWidth: '80%', + maxWidth: SNACKBAR_CONSTANTS.MAX_WIDTH.MD, }, [theme.breakpoints.down('sm')]: { - maxWidth: '100%', + maxWidth: SNACKBAR_CONSTANTS.MAX_WIDTH.SM, }, }))packages/design-system/providers/SnackbarProvider/ProgressBar/styled.tsx (1)
18-34
: Consider adding animation easing for smoother progress.The animation implementation is good, but could be enhanced with easing for a more polished look.
Apply this diff to add easing:
- animation: `increase-width ${animationTime}ms linear forwards`, + animation: `increase-width ${animationTime}ms cubic-bezier(0.4, 0, 0.2, 1) forwards`,packages/utils/hooks/useNotification/index.tsx (1)
16-24
: Consider adding duration configuration.The toast and snack notifications could benefit from configurable durations.
Apply this diff to add duration support:
type NotificationOptions = { type?: string + duration?: number } - sendSnack: (message, { type = 'success' } = {}) => + sendSnack: (message, { type = 'success', duration = 3000 } = {}) => set({ message, type, open: true, shouldShowProgress: true }), - sendToast: (message, { type = 'success' } = {}) => + sendToast: (message, { type = 'success', duration = 2000 } = {}) => set({ message, type, open: true, shouldShowProgress: false }),packages/design-system/providers/SnackbarProvider/__storybook__/Snackbar.mdx (1)
10-10
: Fix grammar in documentation.Add a comma for better readability:
-The messages disappear automatically after a fixed amount of time, but can also be dismissed before by clicking on a close icon. The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message, if used with bar the message is always dismissed after the timeout, no matter whether the user interacted with it or not. +The messages disappear automatically after a fixed amount of time, but can also be dismissed before by clicking on a close icon. The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message, if used with bar, the message is always dismissed after the timeout, no matter whether the user interacted with it or not.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...nteracts with the message, if used with bar the message is always dismissed after t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/components/.storybook/main.ts
(1 hunks)packages/components/modules/messages/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx
(3 hunks)packages/design-system/.storybook/main.ts
(1 hunks)packages/design-system/components/icons/ErrorIcon/index.tsx
(1 hunks)packages/design-system/components/icons/index.ts
(1 hunks)packages/design-system/providers/SnackbarProvider/ProgressBar/index.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/ProgressBar/styled.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/ProgressBar/types.ts
(1 hunks)packages/design-system/providers/SnackbarProvider/__storybook__/Snackbar.mdx
(1 hunks)packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/constants.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/index.tsx
(2 hunks)packages/design-system/providers/SnackbarProvider/styled.tsx
(1 hunks)packages/utils/hooks/useNotification/constants.ts
(1 hunks)packages/utils/hooks/useNotification/index.tsx
(1 hunks)packages/utils/hooks/useNotification/types.ts
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/design-system/providers/SnackbarProvider/__storybook__/Snackbar.mdx
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...nteracts with the message, if used with bar the message is always dismissed after t...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🪛 GitHub Actions: Main Workflow
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx
[error] 4-4: Cannot find module '@baseapp-frontend/utils/dist/hooks/useNotification/types' or its corresponding type declarations.
🔇 Additional comments (19)
packages/components/modules/messages/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx (1)
5-5
: LGTM! Clean integration of notification hook.The import and hook usage are well-organized and follow the project's patterns.
Also applies to: 22-22
packages/design-system/providers/SnackbarProvider/index.tsx (4)
3-11
: New imports look good.
All newly introduced imports appear relevant to the added functionalities (progress bar, theming, etc.). Nicely done.
26-45
: Good fallback for non-progress notifications.
By returning early, you ensure the legacy or simple Snackbar usage remains functional without extra overhead. This is an elegant way to separate the two flows.
46-53
: Manual timeout logic is clear but needs safe cleanup.
Using your own timeout to align progress animation with closure is a solid approach, but be aware of edge cases if the user dismisses the Snackbar early. Clearing the timeout (as mentioned in the previous comment) will ensure correctness.
80-82
: Progress animation integration.
Displaying the progress bar with the correctanimationTime
matches the manual timeout approach. Nicely coordinated.packages/utils/hooks/useNotification/constants.ts (1)
5-5
: New property ensures clarity.
AddingshouldShowProgress
and defaulting it tofalse
provides a clear toggle for progress-based notifications. This is a clean, minimal change.packages/design-system/providers/SnackbarProvider/ProgressBar/types.ts (1)
1-10
: Well-structured type definitions for progress animation.
Defining separate props for severity and animation time is a clear, flexible approach. Merging them intoProgressAnimationProps
looks neat and future-proof.packages/design-system/providers/SnackbarProvider/ProgressBar/index.tsx (1)
6-10
: LGTM! Clean and well-structured component implementation.The ProgressAnimation component is well-implemented with proper TypeScript typing and follows React best practices.
packages/utils/hooks/useNotification/types.ts (2)
1-6
: LGTM! Well-structured type definitions.The NotificationState type is well-defined with proper typing for all properties.
8-13
: LGTM! Comprehensive notification functions interface.The NotificationFunctions type properly defines all necessary methods with appropriate parameter types.
packages/design-system/providers/SnackbarProvider/ProgressBar/styled.tsx (1)
6-16
: LGTM! Well-structured container component.The
ProgressContainer
component is well-implemented with proper flex layout and dynamic background color based on severity.packages/design-system/components/icons/index.ts (1)
12-12
: LGTM! Export follows consistent pattern.The ErrorIcon export is correctly placed in alphabetical order and follows the established export pattern.
packages/design-system/components/icons/ErrorIcon/index.tsx (1)
5-40
: LGTM! Well-implemented icon component.The ErrorIcon component is well-structured with proper typing, theming support, and consistent SVG attributes.
packages/utils/hooks/useNotification/index.tsx (1)
14-15
: LGTM! New snack notification method aligns with requirements.The
sendSnack
method correctly implements the timed snack-bar requirement with progress indicator.packages/design-system/.storybook/main.ts (1)
16-18
: LGTM! Configuration updated to include provider stories.The added story paths correctly extend Storybook's configuration to include documentation and stories from the providers directory.
packages/components/.storybook/main.ts (1)
17-17
: LGTM! Configuration updated to include provider stories.The added story paths correctly extend Storybook's configuration to include documentation and stories from the design-system providers directory.
Also applies to: 23-26
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx (2)
17-25
: LGTM! Well-structured component with clear props interface.The SnackbarWrapper component effectively demonstrates the usage of both toast and snack notifications.
42-132
: LGTM! Comprehensive story coverage.The stories provide excellent coverage of different notification scenarios:
- All notification types (info, success, warning, error)
- Both progress and non-progress variants
- Long message handling
packages/design-system/providers/SnackbarProvider/__storybook__/Snackbar.mdx (1)
19-45
: LGTM! Clear and practical example usage.The example effectively demonstrates how to:
- Set up the SnackbarProvider
- Use both toast and snack notifications
- Handle different notification types
...ges/components/modules/messages/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx
Outdated
Show resolved
Hide resolved
...ges/components/modules/messages/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx
Outdated
Show resolved
Hide resolved
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx
Outdated
Show resolved
Hide resolved
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 (3)
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx (3)
16-24
: Add ARIA label to improve button accessibility.The button should have an aria-label to better describe its action for screen readers.
- <Button onClick={() => send(message, { type })}>Post message</Button> + <Button + onClick={() => send(message, { type })} + aria-label={`Show ${type} notification`} + > + Post message + </Button>
26-39
: Enhance Storybook documentation with component description.Consider adding a description to the meta configuration to provide context about the component's purpose and usage.
const meta: Meta<SnackbarProps> = { title: '@baseapp-frontend | designSystem/SnackbarProvider/SnackbarProvider', component: SnackbarWrapper, + parameters: { + docs: { + description: { + component: 'A provider component that displays toast and snack notifications with optional progress indicators.' + } + } + }, argTypes: {
41-131
: Consider adding edge case stories.The current stories provide good coverage, but consider adding these edge cases:
- Empty message handling
- HTML content in messages
- Very short messages
- Messages with special characters
Example implementation:
export const EdgeCases: Story = { render: (args) => ( <> <SnackbarWrapper {...args} message="" /> <SnackbarWrapper {...args} message="<b>HTML</b> content" /> <SnackbarWrapper {...args} message="Hi!" /> <SnackbarWrapper {...args} message="Special chars: &<>\"'" /> </> ), args: { shouldShowProgress: true, type: 'info' } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/index.tsx
(2 hunks)packages/design-system/providers/SnackbarProvider/styled.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/design-system/providers/SnackbarProvider/styled.tsx
- packages/design-system/providers/SnackbarProvider/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Applications and Packages
- GitHub Check: Lint
🔇 Additional comments (1)
packages/design-system/providers/SnackbarProvider/__storybook__/stories.tsx (1)
1-14
: LGTM! Clean imports and well-defined types.The imports are properly organized and the SnackbarProps interface is well-structured. The NotificationState import path has been correctly updated.
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/utils/index.ts (1)
28-28
: Consider moving type export to the types section.While the export statement is correct, consider moving it to the types section (lines 32-38) for better code organization and consistency.
export { default as useSSR } from './hooks/useSSR' export { default as useBoolean } from './hooks/useBoolean' -export type * from './hooks/useNotification/types' export type * from './types/date' export type * from './types/django' export type * from './types/env' export type * from './types/form' export type * from './types/jwt' export type * from './types/server' +export type * from './hooks/useNotification/types' export type * from './types/typescript'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/utils/index.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Applications and Packages
- GitHub Check: Lint
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 (1)
packages/design-system/providers/SnackbarProvider/index.tsx (1)
19-25
: Consider adding defensive checks for rapid handleClose calls.While the timeout handling is generally correct, consider adding defensive checks for edge cases where handleClose might be called multiple times rapidly.
const handleClose = (_event?: SyntheticEvent | Event, reason?: string) => { if (reason === 'clickaway') { return } - closeToast() - if (timeoutID.current) clearTimeout(timeoutID.current) + if (timeoutID.current) { + clearTimeout(timeoutID.current) + timeoutID.current = null + } + closeToast() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/design-system/providers/SnackbarProvider/ProgressBar/styled.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/constants.tsx
(1 hunks)packages/design-system/providers/SnackbarProvider/index.tsx
(2 hunks)packages/design-system/providers/SnackbarProvider/styled.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/design-system/providers/SnackbarProvider/styled.tsx
- packages/design-system/providers/SnackbarProvider/constants.tsx
- packages/design-system/providers/SnackbarProvider/ProgressBar/styled.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Applications and Packages
- GitHub Check: Lint
🔇 Additional comments (2)
packages/design-system/providers/SnackbarProvider/index.tsx (2)
15-17
: LGTM! Timeout handling implemented correctly.The implementation follows the suggested approach of storing the timeout ID in a ref for proper cleanup.
31-43
: LGTM! Snackbar implementations align with requirements.The implementation successfully:
- Differentiates between regular and progress-enabled notifications
- Uses correct positioning as per requirements
- Integrates progress animation
- Maintains consistent styling with theme
Also applies to: 59-85
fa8fa44
to
2fd0311
Compare
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 (3)
packages/design-system/providers/web/SnackbarProvider/constants.tsx (1)
5-10
: Consider standardizing color definitions across all alert types.While the error and info icons have explicit color definitions, success and warning icons don't specify colors. This could lead to inconsistent styling.
Consider applying this diff to maintain consistency:
error: <ErrorIcon sx={{ color: 'error.main', width: '24px', height: '24px' }} />, info: <Iconify icon="eva:info-outline" width={24} sx={{ color: 'text.primary' }} />, - success: <Iconify icon="eva:checkmark-circle-2-outline" width={24} />, - warning: <Iconify icon="eva:alert-triangle-outline" width={24} />, + success: <Iconify icon="eva:checkmark-circle-2-outline" width={24} sx={{ color: 'success.main' }} />, + warning: <Iconify icon="eva:alert-triangle-outline" width={24} sx={{ color: 'warning.main' }} />,packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx (2)
10-10
: Add comma for better readability.Add a comma to improve the sentence structure:
- The messages disappear automatically after a fixed amount of time, but can also be dismissed before by clicking on a close icon. The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message, if used with bar the message is always dismissed after the timeout, no matter whether the user interacted with it or not. + The messages disappear automatically after a fixed amount of time, but can also be dismissed before by clicking on a close icon. The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message, if used with bar, the message is always dismissed after the timeout, no matter whether the user interacted with it or not.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...nteracts with the message, if used with bar the message is always dismissed after t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
14-15
: Enhance props documentation with type information.Consider adding more detailed type information and default values for props to improve documentation.
- **children** (ReactNode): The content wrapped by the SnackbarProvider. Any components in this wrapped content can make use of 'useNotification' to display messages - **...other**: All other props are passed to the snackbar + **children** (`ReactNode`, required): The content wrapped by the SnackbarProvider. Any components in this wrapped content can make use of 'useNotification' to display messages + **anchorOrigin** (`{ vertical: 'top' | 'bottom', horizontal: 'left' | 'center' | 'right' }`, optional): Position of the snackbar. Default: `{ vertical: 'bottom', horizontal: 'center' }` + **...other**: All other props are passed to the underlying MUI Snackbar component
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx
(1 hunks)packages/design-system/providers/web/SnackbarProvider/constants.tsx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...nteracts with the message, if used with bar the message is always dismissed after t...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and Lint Packages
🔇 Additional comments (1)
packages/design-system/providers/web/SnackbarProvider/constants.tsx (1)
4-4
: LGTM! Duration aligns with UX best practices.The 3-second duration provides sufficient time for users to read notifications while maintaining a snappy UI.
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.
good job on the animation :)
|
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 (3)
packages/utils/CHANGELOG.md (1)
3-8
: LGTM! Consider adding more details about the copy-to-clipboard feature.The changelog entry for version 3.1.5 correctly documents the snackbar progress bar feature. However, since this PR also implements the copy-to-clipboard functionality with toast notifications, consider adding this information to provide a complete record of changes.
Add a new entry under the patch changes section:
## 3.1.5 ### Patch Changes - Implement a snackbar component with a 'progress bar' indicating the remaining time before it automatically disappears +- Add copy-to-clipboard functionality with toast notifications for messages
packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx (2)
10-10
: Fix grammar in the behavior description.Add a comma to improve readability and fix the grammar.
-The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message, if used with bar the message is always dismissed after the timeout, no matter whether the user interacted with it or not. +The component can be used with or without a bar indicating the remaining time before the message is dismissed. If used without a bar, the time until automatic dismissal is restarted after the user interacts with the message. If used with a bar, the message is always dismissed after the timeout, no matter whether the user interacted with it or not.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A comma might be missing here.
Context: ...nteracts with the message, if used with bar the message is always dismissed after t...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
20-46
: Enhance the example code.The example code could be improved in several ways:
- Missing Button import
- No error handling demonstration
- No TypeScript types
```javascript -import { SnackbarProvider } from '@baseapp-frontend/design-system/web' -import { useNotification } from '@baseapp-frontend/utils' +import { Button, SnackbarProvider } from '@baseapp-frontend/design-system/web' +import { useNotification, type ToastOptions } from '@baseapp-frontend/utils' -const MessageEmitter = () => { +const MessageEmitter: React.FC = () => { const { sendToast } = useNotification() + const handleClick = (message: string, options: ToastOptions) => { + try { + sendToast(message, options) + } catch (error) { + console.error('Failed to show toast:', error) + } + } + return ( <> - <Button onClick={() => sendToast("Button 1 was clicked", {type: "info"})}> + <Button onClick={() => handleClick("Button 1 was clicked", {type: "info"})}> Click this button to display a message without progress bar. </Button> - <Button onClick={() => sendToast("Button 2 was clicked", {type: "info", shouldShowProgress: true})}> + <Button onClick={() => handleClick("Button 2 was clicked", {type: "info", shouldShowProgress: true})}> Click this button to display a message with progress bar. </Button> </> ) } -const MyComponent = () => { +const MyComponent: React.FC = () => { return ( <SnackbarProvider> <MessageEmitter /> </SnackbarProvider> ) }</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4e02fef9ecb4e149eda31f232a8cfacc9d920668 and f44bb475f73c65080579a3924d2fd979ac19bcd7. </details> <details> <summary>📒 Files selected for processing (21)</summary> * `packages/authentication/CHANGELOG.md` (1 hunks) * `packages/authentication/package.json` (1 hunks) * `packages/components/CHANGELOG.md` (1 hunks) * `packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx` (3 hunks) * `packages/components/package.json` (1 hunks) * `packages/design-system/CHANGELOG.md` (1 hunks) * `packages/design-system/package.json` (1 hunks) * `packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx` (1 hunks) * `packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx` (1 hunks) * `packages/design-system/providers/web/SnackbarProvider/index.tsx` (2 hunks) * `packages/design-system/providers/web/SnackbarProvider/types.ts` (1 hunks) * `packages/graphql/CHANGELOG.md` (1 hunks) * `packages/graphql/package.json` (1 hunks) * `packages/provider/CHANGELOG.md` (1 hunks) * `packages/provider/package.json` (1 hunks) * `packages/utils/CHANGELOG.md` (1 hunks) * `packages/utils/hooks/useNotification/index.tsx` (1 hunks) * `packages/utils/hooks/useNotification/types.ts` (1 hunks) * `packages/utils/package.json` (1 hunks) * `packages/wagtail/CHANGELOG.md` (1 hunks) * `packages/wagtail/package.json` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (11)</summary> * packages/components/package.json * packages/utils/package.json * packages/graphql/package.json * packages/wagtail/package.json * packages/authentication/package.json * packages/provider/package.json * packages/graphql/CHANGELOG.md * packages/provider/CHANGELOG.md * packages/design-system/package.json * packages/wagtail/CHANGELOG.md * packages/authentication/CHANGELOG.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * packages/utils/hooks/useNotification/types.ts * packages/utils/hooks/useNotification/index.tsx </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>packages/design-system/providers/web/SnackbarProvider/__storybook__/Snackbar.mdx</summary> [uncategorized] ~10-~10: A comma might be missing here. Context: ...nteracts with the message, if used with bar the message is always dismissed after t... (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA) --- [typographical] ~15-~15: Usually, there’s no comma before “if”. Context: ...'shouldShowProgress' option of sendToast, if it is set. - **...other**: All other pr... (IF_NO_COMMA) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (2)</summary> * GitHub Check: Analyze (javascript) * GitHub Check: Build and Lint Packages </details> <details> <summary>🔇 Additional comments (19)</summary><blockquote> <details> <summary>packages/design-system/CHANGELOG.md (2)</summary> `7-7`: **LGTM! The snackbar implementation aligns with PR requirements.** The addition of a snackbar component with a progress bar indicator matches the PR objectives for implementing timed toast notifications. --- `8-9`: **Verify the dependency update.** The dependency update to `@baseapp-frontend/utils@3.1.5` needs to be verified for compatibility. Run this script to check the dependency version: <details> <summary>❌ Verification inconclusive</summary> 🏁 Script executed: ```shell #!/bin/bash # Description: Verify the dependency version exists and check for any breaking changes # Check if the version exists in the registry npm view @baseapp-frontend/utils@3.1.5 version # Check the changelog for breaking changes gh api repos/silverlogic/baseapp-frontend/contents/packages/utils/CHANGELOG.md --ref main | jq -r '.content' | base64 -d | grep -A 10 "## 3.1.5"
Length of output: 2043
Dependency Update Verification Failed for @baseapp-frontend/utils@3.1.5
- The npm command returned a 404 error, indicating that version 3.1.5 is not available in the npm registry.
- The attempt to fetch the changelog via the GitHub API also failed due to command flag and tool issues.
It appears that the dependency update reference in the CHANGELOG is pointing to a version that isn’t published. Please verify if 3.1.5 was meant to be released or if an incorrect version was documented.
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx (2)
9-9
: LGTM! Import added for notification functionality.
The import is correctly placed and necessary for implementing the toast notification feature.
25-25
: LGTM! Hook usage follows best practices.
The useNotification hook is correctly implemented at the component level.
packages/components/CHANGELOG.md (1)
3-12
: LGTM! Well-structured changelog entry.
The changelog entry follows the conventional commit format and clearly documents the new snackbar component with progress bar feature along with dependency updates.
packages/design-system/providers/web/SnackbarProvider/index.tsx (7)
3-3
: Consolidated imports look good.
These new or updated imports cleanly organize required dependencies and modules without issues.
Also applies to: 7-7, 9-11
14-18
: Prop destructuring is concise and purposeful.
Destructuring shouldShowProgress
here is well-organized, ensuring clarity in how props are passed down.
19-25
: Consistent usage of the notification hook.
Pulling shouldShowProgress
from the hook aligns with the new progress feature and keeps logic centralized in one place.
26-37
: Ref-based timeout approach is appropriate.
Storing the timeout ID in a ref ensures that you can clear it properly without unnecessary re-renders.
39-55
: Custom hide logic is well-implemented.
This effect sets up a manual timer for closing the snackbar when displaying a progress bar, ensuring proper lifecycle management by clearing the timer on unmount or close.
57-76
: Fallback scenario without progress is handled smoothly.
Using the standard auto-hide approach with MUI’s autoHideDuration
is straightforward and keeps non-progress usage consistent.
85-106
: Progress-based snackbar implementation is user-friendly.
Rendering a standard Alert with a custom progress animation visually conveys time remaining, enhancing the user experience.
packages/design-system/providers/web/SnackbarProvider/types.ts (1)
5-6
: Optional shouldShowProgress
extends flexibility.
Including this boolean in the type definition cleanly supports the new progress bar feature without any breaking changes.
packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx (6)
1-4
: Essential imports for React and custom utilities.
Importing useNotification
and associated types from your utilities is aligned with your new progress requirement.
5-9
: MUI and Storybook imports are straightforward.
No issues with adding these necessary dependencies for the UI and Storybook environment.
10-14
: Local interface adds clear type safety.
Defining SnackbarProps
with shouldShowProgress
fosters clarity in story configurations.
16-23
: SnackbarWrapper
usage is concise.
Wrapping the Button and provider is a nice approach to demonstrate sending notifications via Storybook.
25-38
: Storybook metadata assures structured stories.
Setting up meta
with arg types for the type
property makes the story interactive and easy to test.
40-130
: Comprehensive story variations.
The multiple “with progress” and “without progress” stories thoroughly showcase snackbar behavior for various message lengths and notification types.
...components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/MessageItem/index.tsx
Show resolved
Hide resolved
* BA-2205 * BA-2205 More timeout to effect * BA-2205 fix rebase errors * BA-2205 shouldShowProgress option * BA-2205 Versioning
* BA-2205 * BA-2205 More timeout to effect * BA-2205 fix rebase errors * BA-2205 shouldShowProgress option * BA-2205 Versioning
* BA-2205 * BA-2205 More timeout to effect * BA-2205 fix rebase errors * BA-2205 shouldShowProgress option * BA-2205 Versioning
* BA-2205 * BA-2205 More timeout to effect * BA-2205 fix rebase errors * BA-2205 shouldShowProgress option * BA-2205 Versioning
Instructions for Testing
Acceptance Criteria
Given I hover my mouse over a message, when the overlay 3 dots menu appears and I click on it, then I should see a copy option in the menu. (Should already be implemented)
Given I click the copy option then, the text of that message should be copied into my clipboard
A toast message should appear notifying that the message has been copied into the clipboard.
Use the existing info icon, not the copy icon for now.
Make the component as similar as the design as possible.
Use the new timed snack-bar component: https://www.figma.com/design/XRD6wSl1m8Kz6XUcAy5CLp/BaseApp---WEB?node-id=7906-140731&t=o0AZWRQITPLFXZQt-0
Include storybook documentation
Guardrail
The snackbar will not have the action button for now
Notes
https://github.com/silverlogic/baseapp-frontend/blob/master/packages/design-system/providers/SnackbarProvider/index.tsx#L33
Approvd
https://app.approvd.io/silverlogic/BA/stories/38644
Summary by CodeRabbit
New Features
Documentation
Chores