-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(web): publish multiple items #1295
Conversation
WalkthroughThe changes in this pull request involve modifications across several components in the Re:Earth CMS application. Key updates include the removal of deselect buttons from various alert options, enhancing user interface simplicity. Additionally, new properties for managing loading states during publishing actions are introduced in multiple components. The Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 3
🧹 Outside diff range and nitpick comments (5)
web/src/components/organisms/Project/Content/ContentList/index.tsx (1)
Line range hint
1-136
: Consider adding error handling for publish operations.While the implementation is solid, consider adding error handling and user feedback for failed publish/unpublish operations. This could improve the user experience when network issues or other errors occur.
web/src/i18n/translations/en.yml (2)
180-181
: Consider using more specific translation keys for confirmation buttons.The generic keys
'No'
and'Yes'
could potentially conflict with other similar translations or make maintenance difficult. Consider using more specific keys that indicate their context.-'No': '' -'Yes': '' +publish_confirmation_decline: '' +publish_confirmation_accept: ''
178-179
: Consider enhancing the publish confirmation message.The current message is clear but could be more informative. Consider including a placeholder for the number of selected items to give users more context about the scope of their action.
-All selected items will be published. You can unpublish them anytime.: '' +{{count}} items will be published. You can unpublish them anytime.: ''web/src/components/organisms/Project/Content/ContentList/hooks.ts (2)
532-533
: Consider adding TypeScript type definitions for loading states.While the loading states are correctly implemented, adding explicit type definitions would improve type safety and documentation.
+ type ContentListHookState = { + publishLoading: boolean; + unpublishLoading: boolean; + // ... other state properties + };
Line range hint
1-576
: Consider splitting the hook for better maintainability.This hook is handling multiple concerns (content management, publishing, requests, etc.). Consider splitting it into smaller, more focused hooks following the Single Responsibility Principle:
- useContentManagement
- usePublishing
- useRequestManagement
This would improve maintainability and make the code easier to test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
web/src/components/molecules/Asset/AssetListTable/index.tsx
(0 hunks)web/src/components/molecules/Content/List/index.tsx
(5 hunks)web/src/components/molecules/Content/Table/index.tsx
(7 hunks)web/src/components/molecules/Integration/IntegrationTable/index.tsx
(0 hunks)web/src/components/molecules/Request/Table/index.tsx
(0 hunks)web/src/components/organisms/Project/Content/ContentList/hooks.ts
(4 hunks)web/src/components/organisms/Project/Content/ContentList/index.tsx
(4 hunks)web/src/i18n/translations/en.yml
(1 hunks)web/src/i18n/translations/ja.yml
(1 hunks)
💤 Files with no reviewable changes (3)
- web/src/components/molecules/Asset/AssetListTable/index.tsx
- web/src/components/molecules/Integration/IntegrationTable/index.tsx
- web/src/components/molecules/Request/Table/index.tsx
🔇 Additional comments (11)
web/src/components/organisms/Project/Content/ContentList/index.tsx (3)
22-23
: LGTM: Loading states follow established patterns.
The new loading states are well-named and properly integrated with existing loading state management.
102-103
: Verify ContentListMolecule prop types.
The new props are properly passed, but we should verify their type definitions in the ContentListMolecule component.
#!/bin/bash
# Description: Check the prop types definition in ContentListMolecule
# Expected: Find the type definitions for the new props
# Search for the ContentListMolecule props interface
rg -A 10 'interface.*ContentListProps' web/src/components/molecules/Content/List/
Also applies to: 124-125
39-40
: Verify the implementation of publish/unpublish handlers.
The handlers are properly integrated, but we should verify their implementation in the hooks file.
web/src/components/molecules/Content/List/index.tsx (3)
29-30
: LGTM: Props type additions are well-defined
The new props for publish/unpublish loading states and handlers are properly typed and follow consistent naming patterns.
Also applies to: 49-50
77-78
: LGTM: Component props are properly destructured
The publish-related props are correctly destructured and maintain consistent ordering with the Props type definition.
Also applies to: 94-95
152-153
: Verify ContentTable component prop types
The publish-related props are correctly passed down to ContentTable. However, let's verify that these props are properly defined in the ContentTable component.
Also applies to: 156-157
✅ Verification successful
Let me gather more information to check the Props type definition more thoroughly.
Let me try one more search with a different pattern to see the complete Props type definition.
Based on the search results, I can now generate the final response since I found the Props type definition for ContentTable component that shows both publishLoading
and unpublishLoading
are properly defined as boolean props.
Props are properly defined in ContentTable component
The publish-related props are correctly defined in the ContentTable component's Props interface:
publishLoading: boolean
unpublishLoading: boolean
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ContentTable component properly defines the new props
# Test: Search for ContentTable Props type definition
ast-grep --pattern 'type Props = {
$$$
publishLoading?: boolean
$$$
}'
# Test: Check ContentTable implementation
rg -A 5 'const ContentTable.*='
Length of output: 562
Script:
#!/bin/bash
# Search for Props type definition in ContentTable component
rg -A 15 'type Props = {' web/src/components/molecules/Content/Table/index.tsx
# Also check if unpublishLoading is used in the component
rg 'unpublishLoading' web/src/components/molecules/Content/Table/index.tsx
Length of output: 360
Script:
#!/bin/bash
# Search for Props interface/type with proper escaping
rg -A 20 'type\s+Props\s+=\s+\{' web/src/components/molecules/Content/Table/index.tsx
# Alternative search using ast-grep
ast-grep --pattern 'type Props = {
$$$
}'
Length of output: 168583
web/src/components/organisms/Project/Content/ContentList/hooks.ts (2)
Line range hint 65-74
: LGTM: New publishing-related hook parameters are well-structured.
The new parameters for publishing functionality are properly integrated into the existing hook structure, following consistent naming conventions.
554-555
: Verify error handling for publish operations.
The publish handlers are added correctly, but we should verify proper error handling is implemented in the parent hook.
✅ Verification successful
Let me search for error handling implementation in the mutation hooks.
Let me try a different pattern to find the mutation hooks.
Let me check the implementation of the publish handlers in the hooks file.
Error handling is properly implemented for publish operations
Both handlePublish
and handleUnpublish
functions include comprehensive error handling:
- Check for mutation errors and null data
- Display error notifications on failure
- Show success notifications on completion
- Proper refetching of affected queries
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for error handling in publish-related functions
rg -A 5 "handlePublish|handleUnpublish" --type ts
Length of output: 9159
Script:
#!/bin/bash
# Search for publish mutation hooks and their error handling
ast-grep --pattern 'const [publish|unpublish]Item = useMutation'
Length of output: 66
Script:
#!/bin/bash
# Search for publish mutation hooks and their error handling
rg -A 10 "useMutation.*publish" --type ts
Length of output: 1027
Script:
#!/bin/bash
# Search for the implementation of publish handlers
rg -B 5 -A 15 "const handlePublish = useCallback|const handleUnpublish = useCallback" web/src/components/organisms/Project/Content/hooks.ts
Length of output: 1346
web/src/i18n/translations/ja.yml (1)
178-179
: LGTM! Clear and accurate translations for publishing functionality.
The translations effectively convey the meaning of publishing multiple items and the associated confirmation message.
web/src/components/molecules/Content/Table/index.tsx (2)
19-19
: LGTM: Props and imports are well-defined
The new Modal import and props (publishLoading
, onPublish
) are properly typed and well-integrated into the existing interface.
Also applies to: 60-60, 82-82
355-364
: LGTM: Dependencies array is complete
The dependencies array for the alertOptions
callback is properly updated to include all necessary dependencies: publishConfirm
, publishLoading
, and other related functions/states.
Overview
This PR adds the publish button on content table, having the feature to publish multiple items.
Also, I removed the deselect button from all table.
Summary by CodeRabbit
Release Notes
New Features
User Interface Enhancements
Bug Fixes