Ba 2128 fe storybook profile page #197
Conversation
… BA-2128-fe-storybook-profile-page
hpmoreira05
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request primarily refactors several TypeScript files by reordering, reintroducing, or adjusting import statements and formatting within JSX. One configuration file has been updated to adjust task dependencies. The changes do not alter any functional logic or interface structures but standardize import formatting and improve consistency across components and task flows. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Turbo Config
participant B as Build Task
participant S as Storybook Task
T->>B: Trigger Build Task
B-->>T: Build Completed
T->>S: Trigger Storybook Task (after Build)
S-->>T: Storybook Ready
Possibly Related PRs
Suggested Reviewers
Poem
🪧 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 (9)
packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/mockResolvers.ts (1)
1-8: Consider enhancing type safety and documentation.The mock resolver would benefit from explicit type definitions and more descriptive test data.
+import { GraphQLResolveInfo } from 'graphql' + +interface MockResolvers { + Node: () => { + __typename: string + id: string + name: string + isBlockedByMe: boolean + } +} + -export const mockResolvers = { +export const mockResolvers: MockResolvers = { Node: () => ({ __typename: 'Profile', - id: 'profile-1', - name: 'John Doe', + id: 'test-profile-1', + name: 'Test User Profile', isBlockedByMe: false, }), }packages/components/modules/profiles/ProfileComponent/__storybook__/mockResolvers.ts (1)
1-19: Add type definitions and improve test data descriptiveness.The mock resolver would benefit from explicit type definitions and more descriptive test data for Storybook visualization.
+interface ProfileImage { + url: string +} + +interface ProfileUrlPath { + path: string +} + +interface MockResolvers { + Profile: () => { + id: string + name: string + biography: string + image: ProfileImage + bannerImage: ProfileImage + followersCount: number + followingCount: number + isFollowedByMe: boolean + isBlockedByMe: boolean + canChange: boolean + urlPath: ProfileUrlPath + } +} + -export const mockResolvers = { +export const mockResolvers: MockResolvers = { Profile: () => ({ - id: 'profile-1', - name: 'John Doe', - biography: 'Software Engineer at Example Inc.', + id: 'test-profile-1', + name: 'Test User Profile', + biography: 'Test user biography for Storybook visualization', image: { url: 'https://via.placeholder.com/50', }, bannerImage: { url: 'https://via.placeholder.com/1200x300', }, - followersCount: 120, - followingCount: 80, + followersCount: 1000, + followingCount: 500, isFollowedByMe: false, isBlockedByMe: false, canChange: true, - urlPath: { path: '/john-doe' }, + urlPath: { path: '/test-user-profile' }, }), }packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/BlockButtonWithDialogWithQuery/index.tsx (1)
7-25: Enhance component reusability with props.The component could be more flexible by accepting props for the node ID and current profile ID instead of hardcoding them.
+interface BlockButtonWithDialogWithQueryProps { + nodeId?: string + currentProfileId?: string +} + -const BlockButtonWithDialogWithQuery = () => { +const BlockButtonWithDialogWithQuery = ({ + nodeId = 'test-id', + currentProfileId = 'current-user-id', +}: BlockButtonWithDialogWithQueryProps) => { const data = useLazyLoadQuery<Query>( graphql` query BlockButtonWithDialogWithQuery @relay_test_operation { - target: node(id: "test-id") { + target: node(id: $nodeId) { ...BlockToggleFragment } } `, - {}, + { nodeId }, ) return ( <BlockButtonWithDialog target={data.target as BlockToggleFragment$key} - currentProfileId="current-user-id" + currentProfileId={currentProfileId} /> ) }packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/stories.tsx (2)
16-21: Enhance story coverage and descriptions.The stories could benefit from more descriptive names and additional edge cases.
-export const NotBlocked: Story = { +export const DefaultUnblockedState: Story = { - name: 'Not Blocked', + name: 'Default - Profile Not Blocked', parameters: { mockResolvers, }, } -export const Blocked: Story = { +export const BlockedState: Story = { - name: 'Blocked', + name: 'Profile is Blocked by Current User', parameters: { mockResolvers: { Node: () => ({ ...mockResolvers.Node(), isBlockedByMe: true, }), }, }, } +export const LoadingState: Story = { + name: 'Loading State while Blocking/Unblocking', + parameters: { + mockResolvers: { + Node: () => new Promise(resolve => setTimeout(() => resolve(mockResolvers.Node()), 2000)), + }, + }, +}Also applies to: 23-33
7-10: Consider adding component description in meta.Adding a description in the meta object would improve the documentation.
const meta: Meta<typeof BlockButtonWithDialog> = { title: '@baseapp-frontend | components/Profiles/ProfileComponent/BlockButtonWithDialog', component: BlockButtonWithDialogWithQuery, + parameters: { + docs: { + description: { + component: 'A button component that allows users to block/unblock profiles with a confirmation dialog.', + }, + }, + }, }packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponentWithQuery/index.tsx (1)
14-16: Consider parameterizing the profile ID in the GraphQL query.The hardcoded "test-id" in the query limits the component's reusability for different testing scenarios. Consider making it dynamic by using a variable.
- query ProfileComponentWithQuery @relay_test_operation { - profile(id: "test-id") { + query ProfileComponentWithQuery($profileId: ID!) @relay_test_operation { + profile(id: $profileId) {Then update the query variables:
- {}, + { profileId: "test-id" },packages/components/modules/profiles/ProfileComponent/__storybook__/stories.tsx (1)
14-33: Refactor event handlers to reduce code duplication.The keyboard and click event handlers contain duplicate logic. Consider extracting the common functionality into a shared handler.
+const handleShareProfile = (target: HTMLElement) => { + if (target.tagName === 'A') { + e.preventDefault() + } + if (target.innerText === 'Share profile') { + alert('Profile URL copied to clipboard!') + } +} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { - const target = e.target as HTMLElement - if ((e.target as HTMLElement).tagName === 'A') { - e.preventDefault() - } - if (target && target.innerText === 'Share profile') { - alert('Profile URL copied to clipboard!') - } + handleShareProfile(e.target as HTMLElement) } }} onClick={(e) => { - const target = e.target as HTMLElement - if ((e.target as HTMLElement).tagName === 'A') { - e.preventDefault() - } - if (target && target.innerText === 'Share profile') { - alert('Profile URL copied to clipboard!') - } + handleShareProfile(e.target as HTMLElement) }}packages/components/modules/profiles/ProfileComponent/index.tsx (1)
Line range hint
42-55: Consider moving formatFollowCount to a shared utility file.This function could be useful across other components that need to format follower counts. Consider extracting it to a shared utility file for reusability.
packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponent.mdx (1)
35-37: Fix formatting in Related Components section.Remove the extra space after the period to maintain consistent formatting throughout the documentation.
- **Related Components**: - - `BlockButtonWithDialog`: Used within the `ProfileComponent` for blocking profiles. - `FollowToggleButton`: Allows toggling the follow/unfollow state for the profile. + **Related Components**: + - `BlockButtonWithDialog`: Used within the `ProfileComponent` for blocking profiles. + - `FollowToggleButton`: Allows toggling the follow/unfollow state for the profile.🧰 Tools
🪛 LanguageTool
[uncategorized] ~37-~37: Loose punctuation mark.
Context: ...cking profiles. -FollowToggleButton: Allows toggling the follow/unfollow sta...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/__generated__/BlockButtonWithDialogWithQuery.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/ProfileComponentWithQuery.graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (9)
packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/BlockButtonWithDialog.mdx(1 hunks)packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/BlockButtonWithDialogWithQuery/index.tsx(1 hunks)packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/mockResolvers.ts(1 hunks)packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/__storybook__/stories.tsx(1 hunks)packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponent.mdx(1 hunks)packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponentWithQuery/index.tsx(1 hunks)packages/components/modules/profiles/ProfileComponent/__storybook__/mockResolvers.ts(1 hunks)packages/components/modules/profiles/ProfileComponent/__storybook__/stories.tsx(1 hunks)packages/components/modules/profiles/ProfileComponent/index.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/modules/profiles/ProfileComponent/BlockButtonWithDialog/storybook/BlockButtonWithDialog.mdx
🧰 Additional context used
🪛 LanguageTool
packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponent.mdx
[uncategorized] ~37-~37: Loose punctuation mark.
Context: ...cking profiles. - FollowToggleButton: Allows toggling the follow/unfollow sta...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (4)
packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponentWithQuery/index.tsx (1)
24-28: LGTM! Clean implementation of the ProfileComponent wrapper.The component correctly passes down the required props and handles the type casting appropriately.
packages/components/modules/profiles/ProfileComponent/__storybook__/stories.tsx (1)
46-54: LGTM! Comprehensive story coverage.The stories effectively cover the three main profile states (unfollowed, followed, and owner view) with appropriate mock data and configurations.
Also applies to: 56-69, 71-79
packages/components/modules/profiles/ProfileComponent/index.tsx (1)
76-76: LGTM! Navigation change aligns with Next.js best practices.The change from useRouter to href attribute follows Next.js 13+ recommendations for client-side navigation.
packages/components/modules/profiles/ProfileComponent/__storybook__/ProfileComponent.mdx (1)
43-69: LGTM! Clear and comprehensive example usage.The example code effectively demonstrates how to use the component with realistic mock data.
| image: { | ||
| url: 'https://nyc3.digitaloceanspaces.com/baseapp-production-storage/media/user-avatars/c/9/19/resized/50/50/cb95449a94688af33f6e9bb090cf2936.png', | ||
| }, | ||
| bannerImage: { | ||
| url: 'https://developers.elementor.com/docs/assets/img/elementor-placeholder-image.png', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace production URLs with placeholder images.
Using production URLs in test data is not recommended. Consider using placeholder image services or local assets.
image: {
- url: 'https://nyc3.digitaloceanspaces.com/baseapp-production-storage/media/user-avatars/c/9/19/resized/50/50/cb95449a94688af33f6e9bb090cf2936.png',
+ url: 'https://via.placeholder.com/50',
},
bannerImage: {
- url: 'https://developers.elementor.com/docs/assets/img/elementor-placeholder-image.png',
+ url: 'https://via.placeholder.com/1200x300',
},📝 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.
| image: { | |
| url: 'https://nyc3.digitaloceanspaces.com/baseapp-production-storage/media/user-avatars/c/9/19/resized/50/50/cb95449a94688af33f6e9bb090cf2936.png', | |
| }, | |
| bannerImage: { | |
| url: 'https://developers.elementor.com/docs/assets/img/elementor-placeholder-image.png', | |
| }, | |
| image: { | |
| url: 'https://via.placeholder.com/50', | |
| }, | |
| bannerImage: { | |
| url: 'https://via.placeholder.com/1200x300', | |
| }, |
There was a problem hiding this comment.
Remember to add the versioning, but wait until #198 gets merged.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (2)
76-169:⚠️ Potential issueImplement delete functionality and improve empty state handling.
The delete functionality is currently a no-op and the empty state could be more informative.
<ActionsOverlay title="Chat" offsetTop={-12} actions={[/* ... */]} enableDelete - handleDeleteItem={() => {}} + handleDeleteItem={() => { + // TODO: Implement delete functionality + throw new Error('Delete functionality not implemented') + }} isDeletingItem={false} ref={chatCardRef} >Also, improve the empty state:
- <div /> + <Typography variant="caption" color="text.disabled"> + No messages yet + </Typography>
109-111: 💡 Verification agent🧩 Analysis chain
Remove delete functionality until implemented.
The delete functionality is enabled but not implemented. Either implement it or remove it until ready.
🏁 Script executed:
#!/bin/bash # Search for other implementations of handleDeleteItem in the codebase rg "handleDeleteItem" --type typescriptLength of output: 74
Below is a new shell script that re-verifies the usage of
handleDeleteItemin the repository by explicitly searching only in TypeScript/TSX files:Once you run this script, if there’s still no output confirming any other implementation or usage of the delete handler, it confirms that the delete-related props in the ChatRoomItem component are not actually used anywhere. Therefore, as originally indicated, the solution is to remove the currently enabled delete properties until a proper implementation is provided.
Action: Remove Unimplemented Delete Functionality
- File:
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx- Lines: 109–111
enableDelete handleDeleteItem={() => {}} isDeletingItem={false}Since no alternative implementation was found after re-verification, please remove these props from the component until you’re ready to implement the delete functionality.
🏁 Script executed:
[run_scripts] #!/bin/bash # Searching for 'handleDeleteItem' occurrences in .tsx files rg "handleDeleteItem" -g "*.tsx" # Searching for 'handleDeleteItem' occurrences in .ts files rg "handleDeleteItem" -g "*.ts"Length of output: 1109
Action Required: Remove Unimplemented Delete Functionality in ChatRoomItem
- File:
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx- Lines: 109–111
enableDelete handleDeleteItem={() => {}} isDeletingItem={false}The
handleDeleteItemprop currently uses a no-op function, and no proper deletion implementation exists in the codebase for this component—even though similar props are present in other areas (e.g., inActionsOverlayand the Message component, where one occurrence even includes a TODO note). Until a complete and consistent implementation is provided, please remove these props from the ChatRoomItem component to avoid confusion.
🧹 Nitpick comments (2)
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (2)
30-62: Add error handling for fragment data and profile validation.Consider adding error handling for fragment data and validating the profile ID earlier in the component to prevent potential runtime errors.
const ChatRoomItem: FC<ChatRoomItemProps> = ({ roomRef, isCardSelected = false, handleClick, Badge = DefaultBadge, BadgeProps = {}, isInArchivedTab = false, }) => { + const { currentProfile } = useCurrentProfile() + if (!currentProfile?.id) { + return null // or show an error state + } + const lastMessageFragment = useFragment<LastMessageFragment$key>(LastMessageFragment, roomRef) const headerFragment = useFragment<TitleFragment$key>(TitleFragment, roomRef) const unreadMessagesCountFragment = useFragment<UnreadMessagesCountFragment$key>( UnreadMessagesCountFragment, roomRef, ) + + if (!lastMessageFragment || !headerFragment || !unreadMessagesCountFragment) { + return null // or show a loading state + }
63-74: Enhance mutation handling with loading states and error handling.The unreadChat mutation could benefit from loading state tracking and error handling, similar to the archive mutation.
- const unreadChat = useCallback(() => { + const [unreadChat, isUnreadMutationInFlight] = useCallback(() => { commitMutation({ variables: { input: { roomId: roomRef.id, profileId: currentProfile?.id as string, }, }, + onError: (error) => { + console.error('Failed to mark chat as unread:', error) + // Handle error (e.g., show toast notification) + } }) }, [roomRef.id, currentProfile])Then update the action's disabled state:
{ - disabled: hasUnreadMessages, + disabled: hasUnreadMessages || isUnreadMutationInFlight, icon: <UnreadIcon />, label: 'Mark as Unread', onClick: unreadChat, hasPermission: true, closeOnClick: true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx(1 hunks)
🔇 Additional comments (1)
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (1)
1-28: LGTM! Well-organized imports.The imports are logically grouped and all necessary dependencies are correctly imported.
|
|
@hpmoreira05 can you please rebase so we can merge this? |
… BA-2128-fe-storybook-profile-page
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/CHANGELOG.md (1)
5-7: Expand Changelog Entry Details
The current changelog entry (“- create profile page storybook”) is concise; however, consider expanding this description to reference the block/unblock functionality and any associated MDX documentation if applicable. This will help clarify the scope of the new Storybook stories introduced by the PR and better align with the PR objectives.Proposed diff:
- - create profile page storybook + - create profile page storybook with block/unblock functionality and MDX documentation for both the Profile page and BlockButtonWithDialog
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/CHANGELOG.md(1 hunks)packages/components/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Component Test Packages
🔇 Additional comments (1)
packages/components/CHANGELOG.md (1)
3-4: New Version Bump is Clear
The new version header "## 1.0.24" is correctly placed and clearly indicates a new release for the components package. Ensure that all related metadata (e.g., version in package.json) is updated consistently.
|
Acceptance Criteria
Context:
The
Profilepage provides a detailed view of a user profile. This story will document theProfilepage in Storybook, as well as the block/unblock feature.Acceptance Criteria:
Create Storybook Stories:
Profile Page:
Add a Storybook story for the
Profilepage.Ensure the profile link sharing functionality is documented and simulated, showing the feedback provided when the link is copied.
Mock uses to display the "Edit Profile" button.
BlockButtonWithDialog Story:
Add a Storybook story for the
BlockButtonWithDialog.Simulate both states, blocked and unblocked.
Document the
ConfirmDialogwith appropriate title and content for blocking and unblocking, as well as the use of icons to enhance visual clarity.Create MDX Documentation:
Add an MDX file for both
ProfileandBlockButtonWithDialog.Follow the guidelines here https://app.tettra.co/teams/TSL/pages/frontend-documentation-guide
GraphQL Mocking with MSW:
Use MSW to mock the required GraphQL operations, such as loading the profile data and executing the block/unblock mutations.
Mocking Examples:
https://github.com/silverlogic/baseapp-frontend/blob/master/packages/components/modules/comments/CommentsList/__storybook__/mockResolvers.ts
https://github.com/silverlogic/baseapp-frontend/blob/master/packages/components/modules/comments/Comments/__tests__/__mocks__/requests.ts
Approvd
https://app.approvd.io/silverlogic/BA/stories/36580
Summary by CodeRabbit
Refactor
Chores