BA-2086-fe-storybook-design-system-dropzone #181
Conversation
hpmoreira05
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
|
WalkthroughThis pull request focuses on enhancing the documentation and Storybook configuration for the Dropzone component in the design system. The changes include creating a comprehensive MDX documentation file and a Storybook stories file that showcase various use cases and configurations of the Dropzone component. Additionally, the PR updates package versions and changelogs across multiple packages, reflecting the updates to the design system and related dependencies. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 4e2597f05356303248030ad29e089f03c2ec4def and fb0c3d1. ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![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
🧹 Nitpick comments (4)
packages/design-system/components/Dropzone/__storybook__/stories.tsx (2)
23-28: Consider using a more reliable image source.While the implementation is correct, consider using a more reliable image source instead of an external URL that might become unavailable.
- storedImg: 'https://developers.elementor.com/docs/assets/img/elementor-placeholder-image.png', + storedImg: '/assets/placeholder-image.png',
30-36: Improve error state demonstration.While this effectively shows the file size error, consider using a more realistic file size limit and adding a story for unsupported file type errors to fully cover the error states.
...Default.args, - maxFileSize: 0.0001, - subTitle: 'Max. File Size: 0.0001MB', + maxFileSize: 1, + subTitle: 'Max. File Size: 1MB',Consider adding a new story for unsupported file types:
export const UnsupportedFileType: Story = { args: { accept: { 'image/svg+xml': ['.svg'] }, actionText: 'Upload SVG only', maxFileSize: 15, subTitle: 'Only SVG files are allowed', }, }packages/design-system/components/Dropzone/__storybook__/dropzone.mdx (2)
12-21: Consider expanding current usage examples.While the potential usage scenarios are well-described, consider adding more real-world examples from the current codebase to help developers understand where and how the component is being used.
22-56: Consider adding TypeScript type information.The props documentation is comprehensive, but could be enhanced by including TypeScript type definitions for better developer experience.
Example format:
- **accept** (`Record<string, string[]>`): Specifies the types of files allowed (e.g., `{ "image/svg+xml": [".svg"] }`).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/design-system/components/Dropzone/__storybook__/dropzone.mdx(1 hunks)packages/design-system/components/Dropzone/__storybook__/stories.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Main Workflow
packages/design-system/components/Dropzone/__storybook__/stories.tsx
[error] 6-6: The inferred type of 'default' cannot be named without a reference to '.pnpm/@storybook+csf@0.1.13/node_modules/@storybook/csf'. This is likely not portable. A type annotation is necessary.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/design-system/components/Dropzone/__storybook__/stories.tsx (2)
13-21: Well-structured default story implementation.The Default story effectively demonstrates the component's initial state with appropriate SVG file constraints and clear user guidance.
38-48: Excellent implementation of multiple file type support.The story effectively demonstrates the component's flexibility with multiple file types and provides clear user guidance.
packages/design-system/components/Dropzone/__storybook__/dropzone.mdx (2)
1-11: Well-structured component documentation.The introduction effectively communicates the component's purpose and behavior, with clear alignment to the stories implementation.
57-82: Excellent example implementation.The example effectively demonstrates the component's usage with all major props and follows best practices for implementation.
| export default { | ||
| title: '@baseapp-frontend | designSystem/Dropzone/Dropzone', | ||
| component: Dropzone, | ||
| } as Meta<DropzoneProps> |
There was a problem hiding this comment.
Fix type annotation for default export to resolve pipeline error.
Add explicit type annotation to resolve the pipeline error about portable types.
-export default {
+const meta: Meta<DropzoneProps> = {
title: '@baseapp-frontend | designSystem/Dropzone/Dropzone',
component: Dropzone,
-} as Meta<DropzoneProps>
+}
+
+export default meta📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default { | |
| title: '@baseapp-frontend | designSystem/Dropzone/Dropzone', | |
| component: Dropzone, | |
| } as Meta<DropzoneProps> | |
| const meta: Meta<DropzoneProps> = { | |
| title: '@baseapp-frontend | designSystem/Dropzone/Dropzone', | |
| component: Dropzone, | |
| } | |
| export default meta |
🧰 Tools
🪛 GitHub Actions: Main Workflow
[error] 6-6: The inferred type of 'default' cannot be named without a reference to '.pnpm/@storybook+csf@0.1.13/node_modules/@storybook/csf'. This is likely not portable. A type annotation is necessary.
… BA-2086-fe-storybook-design-system-dropzone
hpmoreira05
force-pushed
the
BA-2086-fe-storybook-design-system-dropzone
branch
from
5c402a6 to
8a3fa1a
Compare
There was a problem hiding this comment.
is this path right? designSystem/Dropzone/Dropzone @deboracosilveira ? Also maybe we should name the mdx Dropzone to reflect the exported name
There was a problem hiding this comment.
yes, it's. We can also just use @baseapp-frontend | designSystem/Dropzone. What it does is to create a folder in which we can add more components related to dropzones if we have it in the future. On next cop i want to discuss something related to this and we can make a decision.
hpmoreira05
force-pushed
the
BA-2086-fe-storybook-design-system-dropzone
branch
from
8a3fa1a to
6ba0287
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
packages/components/modules/profiles/Members/MemberListItem/index.tsx (1)
Line range hint
44-52: Add missing searchQuery prop to all MemberItemComponent instances.The
searchQueryprop is only passed to owner components but missing in other instances. This could lead to inconsistent search behavior.Add the searchQuery prop to all instances:
<MemberItemComponent member={memberFragment?.user?.profile} memberRole={memberFragment?.role} status={memberFragment?.status} userId={memberFragment?.user?.id} canChangeMember={data?.canChangeRole || false} + searchQuery={searchQuery} {...memberItemComponentProps} />Also applies to: 57-65, 79-87
packages/components/modules/__shared__/ActionsOverlay/index.tsx (1)
Line range hint
88-89: Use useMediaQuery hook for better window matching.The current implementation of deviceHasHover might cause memory leaks and doesn't handle window resize events.
Use Material-UI's useMediaQuery hook for better window matching:
-const deviceHasHover = - typeof window !== 'undefined' && window.matchMedia('(hover: hover)').matches +const deviceHasHover = useMediaQuery('(hover: hover)')Don't forget to import useMediaQuery:
-import { Box, Divider, Typography } from '@mui/material' +import { Box, Divider, Typography, useMediaQuery } from '@mui/material'packages/components/modules/__shared__/SocialInput/styled.tsx (1)
Use MUI's theme.zIndex system instead of hardcoded value
The hardcoded z-index: 10 is too low and inconsistent with the project's z-index hierarchy. Other components use MUI's theme.zIndex system with values like drawer (1200), modal (1300), and tooltip (1500). For a sticky form, consider using an appropriate theme.zIndex value to prevent stacking context issues.
Example locations using theme.zIndex:
packages/components/modules/navigations/Header/styled.tsx: usestheme.zIndex.appBar + 1packages/components/modules/__shared__/ActionsOverlay/styled.tsx: usestheme.zIndex.drawer(1200)🔗 Analysis chain
Line range hint
3-12: Verify z-index stacking context.While the width addition is good, please verify that the z-index value of 10 is sufficient and follows the project's z-index hierarchy to prevent any stacking context issues.
Run this script to check z-index usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for z-index declarations to verify stacking context rg -i "z-?index:?\s*\d+" --type css --type typescript --type javascriptLength of output: 108
Script:
#!/bin/bash # Search for z-index declarations with various patterns rg -i "z-?index[\"']?:\s*[(\[]?[-\d\w.]" # Also search specifically for theme.zIndex usage rg -i "theme\.zIndex\." # Look for any zIndex constant or type definitions rg -i "zIndex\s*[:=]"Length of output: 2774
🧹 Nitpick comments (15)
packages/design-system/components/icons/DownloadIcon/index.tsx (1)
5-6: Consider using a design token for fontSize.While the implementation is correct, consider replacing the hardcoded fontSize value with a design token to maintain consistency across the design system.
- <SvgIcon sx={{ fontSize: 20, color: 'text.primary', ...sx }} {...props}> + <SvgIcon sx={{ fontSize: theme.typography.iconSize.medium, color: 'text.primary', ...sx }} {...props}>packages/design-system/components/icons/CopyIcon/index.tsx (1)
5-7: Consider accessibility and flexibility improvements.A few suggestions to enhance the component:
- Add aria-label for accessibility
- Consider making the size configurable instead of fixed at 20x20
-const CopyIcon: FC<SvgIconProps> = ({ sx, ...props }) => ( +const CopyIcon: FC<SvgIconProps> = ({ sx, size = 20, ...props }) => ( - <SvgIcon sx={{ fontSize: 20, color: 'text.primary', ...sx }} {...props}> + <SvgIcon + sx={{ fontSize: size, color: 'text.primary', ...sx }} + aria-label="copy" + {...props} + > - <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20" fill="none"> + <svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 20 20" fill="none">packages/components/modules/profiles/Members/MembersList/index.tsx (2)
27-28: Consider debouncing the search operation.The search operation triggers a refetch on every keystroke. Consider implementing debounce to reduce unnecessary API calls and improve performance.
+ import { useCallback } from 'react' + import debounce from 'lodash/debounce' const [isPending, startTransition] = useTransition() const { control, reset, watch } = useForm({ defaultValues: { search: '' } }) + const debouncedSearch = useCallback( + debounce((value: string) => { + startTransition(() => { + refetch({ q: value }) + }) + }, 300), + [refetch] + )
53-55: Add memoization for performance optimization.The visibility check and count calculation are performed on every render. Consider memoizing these values to optimize performance.
- const isOwnerVisible = ownerProfile?.name?.includes(watch('search')) - const resultsCount = isOwnerVisible ? members.length + 1 : members.length + const { isOwnerVisible, resultsCount } = useMemo(() => { + const searchTerm = watch('search') + const ownerVisible = ownerProfile?.name?.includes(searchTerm) + return { + isOwnerVisible: ownerVisible, + resultsCount: ownerVisible ? members.length + 1 : members.length + } + }, [ownerProfile?.name, members.length, watch('search')])packages/components/modules/profiles/Members/MemberItem/index.tsx (1)
26-26: Document the searchQuery prop in JSDoc.Add JSDoc documentation for the new searchQuery prop to improve code maintainability.
+ /** + * @property {string} [searchQuery] - Optional search term to filter members by name + */packages/components/modules/__shared__/ActionsOverlay/ThreeDotsMenuHoverOverlay/types.ts (1)
12-17: Consider enhancing type safety for popover state.While the current implementation works, consider creating a dedicated type for the popover state to improve maintainability and type safety.
type PopoverState = { open: HTMLElement | null onOpen: (event: MouseEvent<HTMLElement>) => void onClose: () => void setOpen: Dispatch<SetStateAction<HTMLElement | null>> } export interface ThreeDotsMenuHoverOverlayProps extends Pick<ActionOverlayProps, 'actions' | 'offsetRight' | 'offsetTop' | 'enableDelete' | 'isDeletingItem'> { handleDeleteDialogOpen: () => void handleClosePopover: () => void popover: PopoverState }packages/components/modules/__shared__/ActionsOverlay/DefaultHoverOverlay/index.tsx (1)
14-14: Consider removing the default value for actions.The default value
actions = []is redundant since you're using optional chaining in the map. Consider making the prop required to enforce proper usage.- actions = [], + actions,packages/components/modules/__shared__/ActionsOverlay/ThreeDotsMenuHoverOverlay/index.tsx (1)
36-54: Consider adding keyboard navigation support.The menu items should be accessible via keyboard navigation for better accessibility.
Add
tabIndex={0}to each MenuItem and handle keyboard events:<MenuItem key={label} onClick={() => { onClick?.() handleClosePopover() }} disabled={disabled} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + onClick?.() + handleClosePopover() + } + }} >packages/components/modules/__shared__/ActionsOverlay/index.tsx (1)
232-238: Consider extracting styles to a styled component.The inline styles for the three dots menu mode could be moved to a styled component for better maintainability.
Create a styled component in the styled.ts file:
export const ThreeDotsContainer = styled(Box)({ width: '100%', display: 'flex', justifyContent: 'flex-end', })Then update the component:
- {...(hoverOverlayMode === HOVER_OVERLAY_MODES.THREE_DOTS_MENU && { - sx: { - width: '100%', - display: 'flex', - justifyContent: 'flex-end', - }, - })} + {...(hoverOverlayMode === HOVER_OVERLAY_MODES.THREE_DOTS_MENU && { + component: ThreeDotsContainer + })}packages/components/modules/messages/MessageUpdate/types.ts (1)
1-11: Consider optimizing import paths.The interface is well-structured with proper typing. However, consider simplifying the relative import paths using path aliases to improve maintainability.
Example with path aliases:
-import { MessageItemFragment$data } from '../../../__generated__/MessageItemFragment.graphql' -import { type SocialInputProps } from '../../__shared__/SocialInput/types' +import { MessageItemFragment$data } from '@generated/MessageItemFragment.graphql' +import { type SocialInputProps } from '@shared/SocialInput/types'packages/components/modules/__shared__/UpdateSubmitActions/index.tsx (1)
Line range hint
7-22: Consider enhancing keyboard navigation.While the component has good accessibility with aria-labels, consider adding a consistent tab order for keyboard navigation.
Add
tabIndexprops to ensure a logical tab order:- <IconButton onClick={handleEditCancel} aria-label={cancelAriaLabel}> + <IconButton onClick={handleEditCancel} aria-label={cancelAriaLabel} tabIndex={1}> <CloseIcon /> </IconButton> - <IconButton type="submit" form={formId} aria-label={ariaLabel} disabled={disabled}> + <IconButton type="submit" form={formId} aria-label={ariaLabel} disabled={disabled} tabIndex={2}> <CheckMarkIcon /> </IconButton>packages/components/modules/messages/graphql/mutations/MessageUpdate.ts (1)
37-43: Enhance error handling for mutation completion.The error handling could be more specific about the type of error and provide more context in the notification.
Consider this improvement:
onCompleted: (response, errors) => { errors?.forEach((error) => { - sendToast(error.message, { type: 'error' }) + sendToast(`Failed to update message: ${error.message}`, { type: 'error' }) }) + if (response.chatRoomEditMessage?.errors?.length) { + response.chatRoomEditMessage.errors.forEach(({ field, messages }) => { + sendToast(`${field}: ${messages.join(', ')}`, { type: 'error' }) + }) + } config?.onCompleted?.(response, errors) },packages/components/modules/messages/MessagesList/MessagesGroup/MessageItem/index.tsx (3)
53-53: Implement missing message actions.There are several unimplemented features marked with TODO comments:
- Copy message functionality
- Download attachments functionality
- Delete message functionality
These features should be implemented to complete the message actions functionality.
Would you like me to help implement these features or create issues to track them?
Also applies to: 70-70, 76-76
33-38: Consider extracting typography styles to a theme or constant.The typography styles are defined inline, which could lead to inconsistency if used in multiple places.
Consider moving these styles to a theme or constant:
+const messageTypographyStyles = { + maxWidth: '100%', + whiteSpace: 'pre-wrap', + wordBreak: 'normal', + overflowWrap: 'anywhere', +} as const; // In the component: - sx={{ - maxWidth: '100%', - whiteSpace: 'pre-wrap', - wordBreak: 'normal', - overflowWrap: 'anywhere', - }} + sx={messageTypographyStyles}
24-43: Consider extracting message content rendering to a separate component.The
renderMessageContentfunction could be extracted to a separate component to improve maintainability and testability.Consider creating a new component:
interface MessageContentProps { isEditMode: boolean; message: MessageType; isOwnMessage: boolean; onCancelEdit: () => void; } const MessageContent: FC<MessageContentProps> = ({ isEditMode, message, isOwnMessage, onCancelEdit, }) => { if (isEditMode) { return <MessageUpdate message={message} onCancel={onCancelEdit} />; } return ( <Typography variant="body2" color={isOwnMessage ? 'text.primary' : 'primary.contrastText'} sx={messageTypographyStyles} > {message?.content} </Typography> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 8a3fa1ab6ff7ea61accfbdcd181b895399ae116f and 6ba02874e942f97e80a05e7f3ecf687e68508f0e.
⛔ Files ignored due to path filters (4)
packages/components/__generated__/MessageUpdateMutation.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/UserMembersListFragment.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/UserMembersListPaginationQuery.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/userMembersListPaginationRefetchable.graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (34)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/__shared__/ActionsOverlay/DefaultHoverOverlay/index.tsx(1 hunks)packages/components/modules/__shared__/ActionsOverlay/DefaultHoverOverlay/types.ts(1 hunks)packages/components/modules/__shared__/ActionsOverlay/ThreeDotsMenuHoverOverlay/index.tsx(1 hunks)packages/components/modules/__shared__/ActionsOverlay/ThreeDotsMenuHoverOverlay/types.ts(1 hunks)packages/components/modules/__shared__/ActionsOverlay/__storybook__/stories.tsx(2 hunks)packages/components/modules/__shared__/ActionsOverlay/constants.ts(1 hunks)packages/components/modules/__shared__/ActionsOverlay/index.tsx(8 hunks)packages/components/modules/__shared__/ActionsOverlay/types.ts(2 hunks)packages/components/modules/__shared__/SocialInput/SubmitActions/types.ts(1 hunks)packages/components/modules/__shared__/SocialInput/styled.tsx(1 hunks)packages/components/modules/__shared__/UpdateSubmitActions/index.tsx(2 hunks)packages/components/modules/__shared__/UpdateSubmitActions/types.ts(1 hunks)packages/components/modules/__shared__/index.ts(1 hunks)packages/components/modules/comments/CommentUpdate/index.tsx(3 hunks)packages/components/modules/messages/MessageUpdate/index.tsx(1 hunks)packages/components/modules/messages/MessageUpdate/types.ts(1 hunks)packages/components/modules/messages/MessagesList/MessagesGroup/MessageItem/index.tsx(2 hunks)packages/components/modules/messages/graphql/mutations/MessageUpdate.ts(1 hunks)packages/components/modules/profiles/Members/MemberItem/index.tsx(2 hunks)packages/components/modules/profiles/Members/MemberItem/types.ts(1 hunks)packages/components/modules/profiles/Members/MemberListItem/index.tsx(3 hunks)packages/components/modules/profiles/Members/MemberListItem/types.ts(1 hunks)packages/components/modules/profiles/Members/MembersList/index.tsx(3 hunks)packages/components/modules/profiles/graphql/queries/UserMembersList.ts(2 hunks)packages/components/package.json(1 hunks)packages/components/schema.graphql(3 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/icons/CopyIcon/index.tsx(1 hunks)packages/design-system/components/icons/DownloadIcon/index.tsx(1 hunks)packages/design-system/components/icons/index.ts(1 hunks)packages/design-system/package.json(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/design-system/components/icons/index.ts
- packages/wagtail/package.json
- packages/design-system/package.json
- packages/components/modules/shared/ActionsOverlay/constants.ts
- packages/components/package.json
- packages/wagtail/CHANGELOG.md
- packages/design-system/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/CHANGELOG.md
🔇 Additional comments (24)
packages/design-system/components/icons/DownloadIcon/index.tsx (3)
1-3: LGTM! Clean and necessary imports.The imports are minimal and properly typed with TypeScript.
7-27: Well-structured SVG implementation!The SVG implementation follows best practices:
- Proper use of currentColor for theming
- Clean path definitions with appropriate rules
- Consistent dimensions with viewBox
30-30: LGTM! Proper export convention.The default export follows the established pattern for icon components in the design system.
packages/design-system/components/icons/CopyIcon/index.tsx (3)
1-4: LGTM! Clean and appropriate imports.The imports are minimal and well-structured, using proper TypeScript types and Material-UI components.
5-46: LGTM! Well-structured component implementation.The component follows Material-UI patterns and properly handles style overrides while maintaining theme integration through currentColor.
48-48: LGTM! Clean export statement.The default export is appropriate for this component.
packages/components/modules/profiles/Members/MemberListItem/types.ts (1)
14-14: LGTM! Clean addition of the search query prop.The optional
searchQueryproperty is correctly typed and follows TypeScript best practices.packages/components/modules/profiles/Members/MemberItem/types.ts (1)
19-19: LGTM! Consistent with MemberListItem types.The optional
searchQueryproperty is correctly typed and maintains consistency with the parent component.packages/components/modules/profiles/graphql/queries/UserMembersList.ts (1)
9-9: LGTM! Well-structured GraphQL query updates.The search query parameter is correctly integrated into the GraphQL queries and fragments, with proper connection filter updates.
Also applies to: 13-14, 26-26, 30-31
packages/components/modules/__shared__/ActionsOverlay/DefaultHoverOverlay/types.ts (1)
3-10: Well-structured interface design!The interface is cleanly designed with:
- Proper use of TypeScript's
Pickutility type- Clear and descriptive handler method names
- Focused selection of required props from the parent interface
packages/components/modules/__shared__/ActionsOverlay/types.ts (1)
4-4: Clean type implementation for overlay modes!Good use of:
- ValueOf utility type for type safety
- Optional prop for backward compatibility
- Proper type imports organization
Also applies to: 8-8, 33-33
packages/components/modules/__shared__/ActionsOverlay/DefaultHoverOverlay/index.tsx (3)
1-50: Add test coverage for the new component.Please ensure test coverage for:
- Delete button rendering and functionality
- Action buttons rendering and click handlers
- Permission-based rendering
- Accessibility attributes
Would you like me to help create the test file with the necessary test cases?
20-20: Great attention to accessibility!Excellent use of aria-labels for improved accessibility:
- Container labeled as "actions overlay"
- Delete button labeled as "delete item"
- Action buttons labeled with their respective labels
Also applies to: 26-26, 42-42
1-50: Clarify the relationship between ActionsOverlay changes and Dropzone component.While the code changes look good, they appear to be modifying the ActionsOverlay module while the PR objectives focus on the Dropzone component. Could you please clarify:
- How these changes relate to the Dropzone component?
- Are these changes prerequisites for the Dropzone implementation?
packages/components/modules/__shared__/ActionsOverlay/__storybook__/stories.tsx (1)
41-62: LGTM! Well-structured story for the three dots menu mode.The story effectively demonstrates the new three dots menu functionality with appropriate props and configuration.
packages/components/modules/__shared__/UpdateSubmitActions/types.ts (1)
1-7: Great addition of accessibility support!The interface rename makes it more generic and reusable. The addition of
cancelAriaLabelimproves accessibility by allowing custom aria labels for the cancel action.packages/components/modules/__shared__/SocialInput/SubmitActions/types.ts (1)
Line range hint
1-14: LGTM! Consistent accessibility improvements.The addition of
cancelAriaLabelmaintains consistency with theUpdateSubmitActionsPropsinterface, ensuring a uniform approach to accessibility across components.packages/components/modules/__shared__/index.ts (1)
12-13: LGTM! Good job on improving component reusability.Moving
UpdateSubmitActionsto the shared module makes it more reusable across different features, following the DRY principle.packages/components/modules/messages/graphql/mutations/MessageUpdate.ts (1)
25-28: Verify the mutation return type.Let's verify that the mutation's return type matches the GraphQL schema.
✅ Verification successful
Return type matches schema and follows React hook patterns
The mutation's return type correctly matches the GraphQL schema and follows React hook conventions. The implementation properly uses:
ChatRoomEditMessageInputas input- Returns a tuple of [mutation function, loading state]
- Handles the
ChatRoomEditMessagePayloadresponse through Relay's generated types🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the mutation return type matches the schema definition # Search for the mutation definition in the schema rg -A 5 "type ChatRoomEditMessagePayload"Length of output: 437
Script:
#!/bin/bash # Search for the mutation definition and implementation rg -A 5 "chatRoomEditMessage" packages/components/schema.graphql # Check the TypeScript implementation ast-grep --pattern 'export const useMessageUpdateMutation = $$$'Length of output: 3370
Script:
#!/bin/bash # Find MessageUpdateMutation type definition rg -t typescript "type MessageUpdateMutation" -A 3 # Find the mutation query cat packages/components/modules/messages/graphql/mutations/MessageUpdate.tsLength of output: 1522
packages/components/modules/comments/CommentUpdate/index.tsx (1)
11-11: LGTM! Good refactoring to use shared component.The change to use
UpdateSubmitActionsfrom the shared module instead ofCommentUpdateSubmitActionsimproves code reusability and maintains consistency across the codebase.Let's verify if other components are consistently using the shared
UpdateSubmitActions:Also applies to: 140-140
✅ Verification successful
Verified: Shared UpdateSubmitActions component is used consistently
The shared component is properly used across the codebase, with both Message and Comment modules utilizing it. No legacy component-specific implementations remain, confirming successful refactoring.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of component-specific submit actions rg -l ".*SubmitActions'$" --type ts --type tsx | grep -v "__shared__"Length of output: 100
Script:
#!/bin/bash # Find all imports containing SubmitActions echo "=== Searching for SubmitActions imports ===" fd -e ts -e tsx | xargs ast-grep --pattern 'import $_ from $path' | grep -i "submitactions" # Find component definitions containing SubmitActions echo -e "\n=== Searching for SubmitActions component definitions ===" fd -e ts -e tsx | xargs ast-grep --pattern 'const $_ = $$$SubmitActions$$$'Length of output: 397
Script:
#!/bin/bash echo "=== Searching for UpdateSubmitActions imports ===" rg -t typescript -t javascriptreact -t typescriptreact "import.*UpdateSubmitActions" -A 1 echo -e "\n=== Searching for any *SubmitActions components ===" rg -t typescript -t javascriptreact -t typescriptreact "SubmitActions" -A 1Length of output: 465
Script:
#!/bin/bash echo "=== Searching for UpdateSubmitActions ===" rg "import.*UpdateSubmitActions" echo -e "\n=== Searching for CommentUpdateSubmitActions ===" rg "CommentUpdateSubmitActions"Length of output: 640
packages/components/modules/messages/MessageUpdate/index.tsx (1)
145-146: Good accessibility implementation!The addition of
ariaLabelandcancelAriaLabelprops improves accessibility for screen readers.packages/components/schema.graphql (3)
166-170: LGTM! Well-structured input type.The
ChatRoomEditMessageInputtype follows GraphQL best practices with required fields properly marked.
172-178: LGTM! Consistent payload type structure.The
ChatRoomEditMessagePayloadtype follows the established pattern with proper error handling and debug information.
695-695: LGTM! Proper mutation integration.The
chatRoomEditMessagemutation is properly integrated into the schema.
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the Searchbar component to reduce duplication.
The Searchbar component configuration is duplicated. Consider extracting it into a separate component or at least a reusable configuration object.
+ const SearchbarComponent = () => (
+ <Searchbar
+ variant="outlined"
+ size="small"
+ isPending={isPending}
+ onChange={(e) => handleSearch(e.target.value)}
+ onClear={() => handleSearchClear()}
+ name="search"
+ control={control}
+ sx={{ mb: 4 }}
+ />
+ )
// Then use it in both places:
<SearchbarComponent />Also applies to: 109-118
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for the search operation.
The search operation lacks error handling for the refetch operation. Consider adding error handling and user feedback.
const handleSearch = (value: string) => {
startTransition(() => {
- refetch({ q: value })
+ refetch({ q: value }).catch((error) => {
+ console.error('Failed to search members:', error)
+ // Add user feedback here (e.g., toast notification)
+ })
})
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance search filtering with proper null checks and type safety.
The current implementation might cause issues with undefined values and case sensitivity. Consider adding proper null checks and improving type safety.
- if (searchQuery && !memberProfile?.name?.toLowerCase().includes(searchQuery.toLowerCase())) {
+ if (searchQuery?.trim() && memberProfile?.name) {
+ const normalizedSearch = searchQuery.trim().toLowerCase()
+ const normalizedName = memberProfile.name.toLowerCase()
+ if (!normalizedName.includes(normalizedSearch)) {
+ return null
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (searchQuery && !memberProfile?.name?.toLowerCase().includes(searchQuery.toLowerCase())) { | |
| return null | |
| } | |
| if (searchQuery?.trim() && memberProfile?.name) { | |
| const normalizedSearch = searchQuery.trim().toLowerCase() | |
| const normalizedName = memberProfile.name.toLowerCase() | |
| if (!normalizedName.includes(normalizedSearch)) { | |
| return null | |
| } | |
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety and null checks in the actions mapping.
Several improvements can be made to the actions mapping logic:
- Remove optional chaining since actions is required
- Add type guard for onClick to prevent undefined calls
- Consider using a type for the label to enforce unique keys
- {actions?.map(({ label, icon, onClick, disabled, hasPermission, closeOnClick }) => {
+ {actions.map(({ label, icon, onClick, disabled, hasPermission, closeOnClick }) => {
if (!hasPermission) return null
const handleClick = () => {
- onClick?.()
+ if (onClick) {
+ onClick()
+ }
if (closeOnClick) {
handleLongPressItemOptionsClose()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {actions?.map(({ label, icon, onClick, disabled, hasPermission, closeOnClick }) => { | |
| if (!hasPermission) return null | |
| const handleClick = () => { | |
| onClick?.() | |
| if (closeOnClick) { | |
| handleLongPressItemOptionsClose() | |
| } | |
| } | |
| return ( | |
| <IconButton key={label} onClick={handleClick} disabled={disabled} aria-label={label}> | |
| {icon} | |
| </IconButton> | |
| ) | |
| })} | |
| {actions.map(({ label, icon, onClick, disabled, hasPermission, closeOnClick }) => { | |
| if (!hasPermission) return null | |
| const handleClick = () => { | |
| if (onClick) { | |
| onClick() | |
| } | |
| if (closeOnClick) { | |
| handleLongPressItemOptionsClose() | |
| } | |
| } | |
| return ( | |
| <IconButton key={label} onClick={handleClick} disabled={disabled} aria-label={label}> | |
| {icon} | |
| </IconButton> | |
| ) | |
| })} |
There was a problem hiding this comment.
Add anchorEl and positioning props to Popover.
The Popover component is missing essential props for proper positioning.
Add these props to ensure correct menu positioning:
- <Popover open={popover.open} onClose={handleClosePopover}>
+ <Popover
+ open={popover.open}
+ onClose={handleClosePopover}
+ anchorEl={popover.anchorEl}
+ anchorOrigin={{
+ vertical: 'bottom',
+ horizontal: 'right',
+ }}
+ transformOrigin={{
+ vertical: 'top',
+ horizontal: 'right',
+ }}
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Popover open={popover.open} onClose={handleClosePopover}> | |
| <Popover | |
| open={popover.open} | |
| onClose={handleClosePopover} | |
| anchorEl={popover.anchorEl} | |
| anchorOrigin={{ | |
| vertical: 'bottom', | |
| horizontal: 'right', | |
| }} | |
| transformOrigin={{ | |
| vertical: 'top', | |
| horizontal: 'right', | |
| }} | |
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error boundary for hover overlay modes.
The current implementation doesn't handle invalid hover overlay modes gracefully.
Add error handling for invalid modes:
if (deviceHasHover && isHoveringItem) {
if (hoverOverlayMode === HOVER_OVERLAY_MODES.DEFAULT) {
return <DefaultHoverOverlay {...props} />
}
if (hoverOverlayMode === HOVER_OVERLAY_MODES.THREE_DOTS_MENU) {
return <ThreeDotsMenuHoverOverlay {...props} />
}
+
+ console.error(`Invalid hover overlay mode: ${hoverOverlayMode}`)
+ return <DefaultHoverOverlay {...props} />
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Implement proper error handling instead of using console.error.
The current implementation uses TODO comments and console.error for error handling. This should be replaced with proper error handling logic.
Consider implementing a proper error handling mechanism:
- // TODO: handle errors
- console.error(errors)
+ const errorMessage = errors.map(error => error.message).join(', ')
+ // Replace with your error handling mechanism, e.g.:
+ notifyError(errorMessage)- // TODO: handle errors
- onError: console.error,
+ onError: (error) => {
+ const errorMessage = error?.message ?? 'An error occurred while updating the message'
+ // Replace with your error handling mechanism, e.g.:
+ notifyError(errorMessage)
+ },Also applies to: 114-115
hpmoreira05
force-pushed
the
BA-2086-fe-storybook-design-system-dropzone
branch
from
6ba0287 to
4e2597f
Compare
… BA-2086-fe-storybook-design-system-dropzone
hpmoreira05
force-pushed
the
BA-2086-fe-storybook-design-system-dropzone
branch
from
4e2597f to
40b57c5
Compare
|
anicioalexandre
deleted the
BA-2086-fe-storybook-design-system-dropzone
branch
Acceptance Criteria
Context:
The
Dropzonecomponent provides a file upload area that allows users to either drag and drop files or click to browse for files. It should be documented in Storybook will demonstrate its functionality.Acceptance Criteria:
Create Storybook Story:
Add a Storybook story for the
Dropzonecomponent.Showcase the following states and functionalities if possible:
Default State: Display the component with no files selected.
File Selected: Show the component after an image has been uploaded, including a preview and a "Remove" button.
File Dragging: Demonstrate the component’s behavior when a file is being dragged over the drop area, including states for accepted and rejected files.
Error State: Show the component with an error message if a file type is not accepted.
Create MDX Documentation:
Add an MDX file for
Dropzone.Follow Documentation Standards:
Ensure both the Storybook story and MDX documentation align with the guidelines in https://app.tettra.co/teams/TSL/pages/frontend-documentation-guide
Summary by CodeRabbit
Release Notes v0.0.51
Documentation
Dependency Updates
@baseapp-frontend/design-systemto version 0.0.34.Chores