feat(web): add user remove button and improve remove users modal #1288
Conversation
WalkthroughThe changes in this pull request introduce a new icon, Changes
Possibly related PRs
Suggested reviewers
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 canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
web/src/components/molecules/Workspace/types.ts (1)
19-19: LGTM! Type simplification improves maintainability.
The change to use the imported User type instead of an inline definition promotes better type reuse and maintainability. This is a good refactoring that follows the DRY principle.
Consider documenting the User type dependencies in a central location (e.g., README.md) to help other developers understand the type hierarchy and relationships.
web/src/components/molecules/Member/MemberTable/index.tsx (1)
297-325: LGTM: Well-structured styled components.
The styled components create a clean and maintainable UI structure with proper handling of text overflow and spacing.
Consider adding a comment above the styled components section to document the component hierarchy:
// Styled components for the remove users modal
// RemoveUsers
// └─ RemoveUser
// ├─ UserAvatar (imported)
// └─ UserInfoWrapper
// ├─ UserInfo
// └─ Emailweb/src/i18n/translations/ja.yml (2)
436-560: Standardize punctuation in system messages.
There's inconsistent usage of periods in system messages. Some messages end with periods while others don't. In Japanese UI messages, it's conventional to omit the period (。) at the end unless it's a complex sentence.
Consider removing trailing periods from messages like:
-Failed to update user.: ユーザーの更新に失敗しました。
+Failed to update user.: ユーザーの更新に失敗しました
-Successfully updated user!: ユーザーの更新に成功しました。
+Successfully updated user!: ユーザーの更新に成功しました227-227: Translate technical terms for better localization.
Some technical terms are left untranslated, which might affect the user experience for Japanese users who are not familiar with English technical terms.
Consider translating:
-Webhook: ''
+Webhook: 'ウェブフック'Also applies to: 228-228, 229-229
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- web/src/components/atoms/Icon/icons.ts (2 hunks)
- web/src/components/molecules/Member/MemberTable/index.tsx (6 hunks)
- web/src/components/molecules/Workspace/types.ts (2 hunks)
- web/src/i18n/translations/en.yml (1 hunks)
- web/src/i18n/translations/ja.yml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/src/i18n/translations/en.yml
🧰 Additional context used
🪛 Biome
web/src/components/molecules/Member/MemberTable/index.tsx
[error] 76-76: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (6)
web/src/components/molecules/Workspace/types.ts (1)
3-3: Verify the imported User type usage across the codebase.
The addition of this import and removal of the local User type definition could impact other components that were using the previous definition.
Let's verify the usage and compatibility:
✅ Verification successful
Let me analyze the results and generate one more verification to ensure type compatibility:
The results will help us verify if there are any incompatibilities between the User type definitions that could cause issues.
Let me try one more time with a corrected regex:
Potential type incompatibility detected between User type definitions
The imported User type from Member/types.ts has fewer fields than the one in AccountSettings/types.ts. However, based on the usage patterns found earlier, the import in Workspace/types.ts correctly uses the Member-specific User type, which is also consistently used across related member management components. The removal of the local User type definition and importing from Member/types.ts is appropriate in this context.
Member/types.tsUser:{ id, name, email }- Used by: Workspace, MemberTable, MemberAddModal, Request, and Members components
- All usages are within the member management context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining local User type definitions and verify usage patterns
# Test 1: Check if there are any remaining local User type definitions that might conflict
rg -g '*.ts' -g '*.tsx' 'type User = \{'
# Test 2: Check usage patterns of User type to ensure compatibility
rg -g '*.ts' -g '*.tsx' -A 3 'import.*User.*from.*Member/types'
Length of output: 2355
Script:
#!/bin/bash
# Check the User type definitions in both locations to verify compatibility
echo "=== User type in Member/types.ts ==="
rg -A 10 "export type User = {" web/src/components/molecules/Member/types.ts
echo -e "\n=== User type in AccountSettings/types.ts ==="
rg -A 10 "export type User = {" web/src/components/molecules/AccountSettings/types.ts
Length of output: 564
Script:
#!/bin/bash
# Check the User type definitions in both locations to verify compatibility
echo "=== User type in Member/types.ts ==="
rg -A 10 '^export type User' web/src/components/molecules/Member/types.ts
echo -e "\n=== User type in AccountSettings/types.ts ==="
rg -A 10 '^export type User' web/src/components/molecules/AccountSettings/types.ts
Length of output: 579
web/src/components/atoms/Icon/icons.ts (2)
8-8: LGTM! Clean import addition.
The UsergroupDeleteOutlined import is properly placed and maintains the file's alphabetical ordering convention.
102-102: LGTM! Well-structured icon mapping.
The new userGroupDelete mapping follows the established naming conventions and is appropriately positioned in the export object.
web/src/components/molecules/Member/MemberTable/index.tsx (3)
14-14: LGTM: Clean type import.
The import of the User type from a centralized location promotes better type consistency across the application.
150-151: LGTM: Well-implemented action column improvements.
The changes effectively handle:
- Appropriate width adjustment for the new remove button
- Proper conditional rendering between remove/leave actions
- Correct permission checks
Also applies to: 173-190
65-90: Verify test coverage for the new modal implementation.
The modal improvements look good, but we should ensure proper test coverage for the new user removal flow.
🧰 Tools
🪛 Biome
[error] 76-76: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
web/src/components/molecules/Member/MemberTable/index.tsx (2)
72-90: Consider improving modal content accessibility.
While the modal content provides good visual feedback, consider these accessibility improvements:
- Add ARIA labels for the user list
- Use semantic HTML elements for the user list (e.g.,
<ul>and<li>)
Apply this diff to improve accessibility:
content: (
<>
- <RemoveUsers>
+ <RemoveUsers role="list" aria-label="Users to be removed">
{users.map(user => (
- <RemoveUser key={user.id}>
+ <RemoveUser key={user.id} role="listitem">
<UserAvatar username={user.name} />
<UserInfoWrapper>
<UserInfo>{user.name}</UserInfo>
<Email>{user.email}</Email>
</UserInfoWrapper>
</RemoveUser>
))}
</RemoveUsers>
- <div>
+ <p>
{t(
"Remove this member from workspace means this member will not view any content of this workspace.",
)}
- </div>
+ </p>
</>
),323-325: Consider using theme variables for colors.
Instead of hardcoding the color value, consider using a theme variable for better maintainability and consistency.
const Email = styled(UserInfo)`
- color: #00000073;
+ color: ${props => props.theme.colors.text.secondary};
`;
Overview
This PR adds the user remove button on the action column and improves remove users modal.
Summary by CodeRabbit
Release Notes
New Features
UsergroupDeleteOutlined, for enhanced visual representation.MemberTablecomponent with detailed user information in the confirmation modal during member deletion.Bug Fixes
Documentation
Chores