-
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-2387: activity log storybook #248
Conversation
|
WalkthroughThis pull request introduces new Storybook documentation and story files for the activity log components. Detailed documentation for the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant ALQ as ActivityLogWithQuery
participant API as GraphQL API
participant AL as ActivityLog
U->>ALQ: Render Activity Log view
ALQ->>API: Execute GraphQL query (useLazyLoadQuery)
API-->>ALQ: Return activity logs data
ALQ->>AL: Pass queryRef with data
AL->>U: Display grouped activity logs
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/activity-log/web/ActivityLogComponent/__storybook__/mockResolvers.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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: 1
🧹 Nitpick comments (11)
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLogWithQuery.tsx (1)
8-12
: Consider adding more information to the Relay test operationThe GraphQL query is correctly marked with
@relay_test_operation
, but you might want to add a descriptive name or comment to clarify its purpose in test environments.packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/stories.tsx (2)
7-10
: Story component and declared type don't matchThe meta object declares
ActivityLog
as the component type, but setsActivityLogWithQuery
as the actual component to render. While this works functionally (since ActivityLogWithQuery renders ActivityLog), it's better to align the type declaration with the actual component being rendered.-const meta: Meta<typeof ActivityLog> = { +const meta: Meta<typeof ActivityLogWithQuery> = { title: '@baseapp-frontend | components/ActivityLog/ActivityLog', component: ActivityLogWithQuery, }
16-21
: Story name inconsistent with componentThe story is named
DefaultLogItem
but it's actually for theActivityLog
component. This could be confusing since there's also a LogItem component in the system. Consider renaming it toDefaultActivityLog
for clarity.export const DefaultLogItem: Story = { - name: 'Default ActivityLog', + name: 'Default ActivityLog', parameters: { mockData, }, }And rename the export as well:
-export const DefaultLogItem: Story = { +export const DefaultActivityLog: Story = { name: 'Default ActivityLog', parameters: { mockData, }, }packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/stories.tsx (1)
36-38
: Consider making avatar URLs consistent across mock dataThe avatar URL uses a generic placeholder service. For consistency with the mock data in other stories and resolvers, consider using the same URL pattern or service across all mock data files.
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/mockResolvers.ts (1)
1-308
: Consider organizing mock data into smaller, focused setsThe mock data file is quite large with many repetitive entries. Consider organizing it into smaller, reusable datasets or extracting helper functions to generate mock entries. This would make the file more maintainable.
// Example approach const createMockUser = (id: string, name: string, email: string, avatarUrl?: string) => ({ id, fullName: name, email, avatar: avatarUrl ? { url: avatarUrl } : null, }); const createMockLogEntry = (id: string, verb: string, createdAt: string, user: any, cursor: string) => ({ node: { id, createdAt, verb, url: '/graphql', user, __typename: 'ActivityLog', }, cursor, }); // Then use these helpers to create your mock data export const mockData = { data: { activityLogs: { edges: [ createMockLogEntry( 'QWN0aXZpdHlMb2c6NWQ1NDA4M2UtZjRkOS00MzQ4LWJmM2ItN2YwMzBiZjgwYThl', 'baseapp_reactions.add_reaction', '2025-03-09T10:11:00.395806+00:00', createMockUser('VXNlcjozOQ==', 'Activity Lok', 'll+act2@tsl.io', 'https://nyc3.digitaloceanspaces.com/baseapp-db-staging/media/profile_images/2162b9f5-3c50-4b45-8d33-9fdb28f5dee4.JPG.48x48_q85.jpg'), 'YXJyYXljb25uZWN0aW9uOjc0' ), // Additional entries... ], pageInfo: { endCursor: 'YXJyYXljb25uZWN0aW9uOjkz', hasNextPage: false, }, }, }, };packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/stories.tsx (1)
14-16
: Story name mismatch with componentThe story is named
DefaultLogItem
but it's demonstrating theLogGroups
component. This naming mismatch can be confusing for developers browsing the Storybook.-export const DefaultLogItem: Story = { - name: 'Default LogGroups', +export const DefaultLogGroups: Story = { + name: 'Default LogGroups',packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLog.mdx (1)
9-9
: Add a comma for improved readabilityThere should be a comma after "activity log groups" to properly separate the clauses in this sentence.
-A component that renders the activity log groups allowing the user to search by user and filter by periods. +A component that renders the activity log groups, allowing the user to search by user and filter by periods.🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...component that renders the activity log groups allowing the user to search by user and...(AI_HYDRA_LEO_MISSING_COMMA)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/LogItem.mdx (1)
10-11
: Redundant "Expected Behavior" entriesThere are two separate entries for "Expected Behavior" which is redundant. Consider combining them or using bullet points to differentiate between the behaviors.
-Expected Behavior**: Displays comment content with the author's avatar and name, allows editing and deleting (if permitted), supports reactions and replies, and can expand/collapse nested replies. -- **Expected Behavior**: Displays single log based on the verb passed to it. +Expected Behavior**: + - Displays comment content with the author's avatar and name, allows editing and deleting (if permitted), supports reactions and replies, and can expand/collapse nested replies. + - Displays single log based on the verb passed to it.packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/LogGroups.mdx (3)
10-10
: Improve grammatical structureThe sentence has a few grammatical issues that should be corrected for clarity.
-Displays array of log groups. A group is intended to group activities happening in close times, instead of just display each one (similar to how Slack does it). +Displays array of log groups. A group is intended to group activities happening at close times, instead of just displaying each one (similar to how Slack does it).🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: The preposition “at” seems more likely in this position than the preposition “in”.
Context: ... intended to group activities happening in close times, instead of just display ea...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_AT)
[uncategorized] ~10-~10: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ppening in close times, instead of just display each one (similar to how Slack does it)...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
18-18
: Correct pluralization in prop descriptionSince you're describing a list of items, "reference" should be plural.
-**logGroups** (LogGroup[]): List of relay fragment reference for log items. +**logGroups** (LogGroup[]): List of relay fragment references for log items.🧰 Tools
🪛 LanguageTool
[uncategorized] ~18-~18: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...** (LogGroup[]): List of relay fragment reference for log items. - LoadingState (FC):...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
52-52
: Component name mismatch in exampleThe example defines a component named
MyComponent
but exportsActivityLogComponent
. This mismatch could cause confusion.-export default ActivityLogComponent +export default MyComponent
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/components/__generated__/ActivityLogsFragment.graphql.ts
is excluded by!**/__generated__/**
📒 Files selected for processing (8)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/LogGroups.mdx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/stories.tsx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/LogItem.mdx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/stories.tsx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLog.mdx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLogWithQuery.tsx
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/mockResolvers.ts
(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/stories.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/stories.tsx (2)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/stories.tsx (1)
DefaultLogItem
(14-66)packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/stories.tsx (1)
DefaultLogItem
(16-21)
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/stories.tsx (2)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/stories.tsx (1)
DefaultLogItem
(14-42)packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/stories.tsx (1)
DefaultLogItem
(14-66)
🪛 LanguageTool
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLog.mdx
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...component that renders the activity log groups allowing the user to search by user and...
(AI_HYDRA_LEO_MISSING_COMMA)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/__storybook__/LogGroups.mdx
[uncategorized] ~10-~10: The preposition “at” seems more likely in this position than the preposition “in”.
Context: ... intended to group activities happening in close times, instead of just display ea...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_AT)
[uncategorized] ~10-~10: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ppening in close times, instead of just display each one (similar to how Slack does it)...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~18-~18: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...** (LogGroup[]): List of relay fragment reference for log items. - LoadingState (FC):...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🔇 Additional comments (4)
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/ActivityLogWithQuery.tsx (1)
7-16
: Well-implemented wrapper component for activity logs!This component correctly wraps the ActivityLog component with a GraphQL query using useLazyLoadQuery. The implementation follows React-Relay patterns by defining a GraphQL fragment query and passing the retrieved data to the underlying component.
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/__storybook__/stories.tsx (1)
14-42
: Well-structured LogItem story with good mock dataThe story provides a complete set of mock data that demonstrates all the important properties of a LogItem. The use of placeholder images and realistic data values makes this a good example for documentation.
packages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/mockResolvers.ts (2)
1-308
: Comprehensive mock data for activity logsThe mock data covers a good range of scenarios with different users, timestamps, and activity types. This will be useful for testing and demonstrating the component's capabilities.
301-305
: Pagination mock is well-structuredThe inclusion of pageInfo with endCursor and hasNextPage properties is excellent - it properly simulates GraphQL pagination which will be important for testing infinite scrolling or "load more" functionality.
...ages/components/modules/activity-log/web/ActivityLogComponent/__storybook__/mockResolvers.ts
Outdated
Show resolved
Hide resolved
|
Storybook for Activity Log components
Loom: https://www.loom.com/share/8596c6e240e7483787ba5ffa07fd208b?sid=07586109-e865-433a-8df8-cf28f89fdfd3
Summary by CodeRabbit
Documentation
LogGroups
,LogItem
, andActivityLogComponent
, detailing usage, props, and examples.New Features
LogGroups
,LogItem
, andActivityLog
, showcasing their functionalities and interactions.ActivityLogWithQuery
for fetching and displaying activity log data dynamically.