feat(web): add account page to change password #1155
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces a comprehensive update to user account management within a React application. It includes new components and hooks for managing user settings, password updates, and user deletion. The changes enhance the user interface with a settings page, a password modal for secure updates, and improved navigation options. 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
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Outside diff range and nitpick comments (14)
web/src/beta/pages/AccountSettingsPage/index.tsx (1)
7-11: LGTM: Rendering logic is sound, but consider prop filtering.The component structure using Page and AccountAndWorkSpaceSetting is well-organized and aligns with the PR objective. The use of a render prop for Page allows for flexible content rendering.
Consider filtering the props passed to AccountAndWorkSpaceSetting to ensure only necessary props are forwarded. This can be achieved by destructuring the required props:
renderItem={({ prop1, prop2, ...otherProps }) => ( <AccountAndWorkSpaceSetting prop1={prop1} prop2={prop2} {...otherProps} tab="account" /> )}This approach can help prevent passing unnecessary props and improve component predictability.
web/src/services/routing/index.tsx (1)
38-41: LGTM: New route for account settings pageThe new route for the account settings page is well-structured and correctly placed among other settings-related routes. The path "settings/account" follows RESTful URL patterns and is consistent with the existing routing structure.
Consider adding a general "settings" route that could serve as a landing page for all settings, including a link to this new account settings page. This could improve navigation and user experience. For example:
{ path: "settings", element: <SettingsLandingPage /> },web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx (1)
1-101: Consider adding account deletion functionality.The implementation of the AccountSetting component looks good overall and fulfills the main objective of allowing users to change their passwords. However, as mentioned in the PR objectives, the functionality to delete an account has not been included.
Consider adding an account deletion feature to fully meet the PR objectives. This could involve:
- Adding a new section in the component for account deletion.
- Implementing a confirmation modal for account deletion to prevent accidental deletions.
- Creating a new API endpoint for account deletion if it doesn't already exist.
- Updating the component props to include an onDeleteAccount function.
Example addition to the component:
<Collapse size="large" title={t("Delete Account")}> <SettingsFields> <Typography>{t("Warning: This action cannot be undone.")}</Typography> <Button onClick={handleDeleteAccountClick} appearance="danger" > {t("Delete Account")} </Button> </SettingsFields> </Collapse> // Add a confirmation modal for delete account actionRemember to handle this sensitive operation with appropriate safeguards and user confirmations.
web/src/beta/lib/reearth-ui/components/IconButton/index.tsx (2)
8-8: LGTM! Consider updating documentation.The additions to
IconButtonPropsenhance the component's flexibility:
- The new "medium" size option provides more granular control over button dimensions.
- The
hasBorderproperty allows for conditional border rendering, improving customization.These changes align well with the component's purpose and the PR objectives.
Consider updating the component's documentation to reflect these new options, ensuring developers are aware of the expanded functionality.
Also applies to: 13-13
62-62: LGTM! Consider a minor refactor for size calculations.The
StyledButtonstyled component has been correctly updated:
- The
hasBorderprop is properly destructured and used for conditional border styling.- Width and height calculations now include the new "medium" size option.
These changes are consistent with the type updates and enhance the component's flexibility.
Consider refactoring the size calculations to reduce code duplication:
const sizeMap = { smallest: "16px", small: "20px", medium: "28px", large: "36px", normal: "24px" }; const buttonSize = sizeMap[size] || "24px"; width: buttonSize, height: buttonSize,This refactoring would make the code more maintainable and less prone to errors when adding or modifying sizes in the future.
Also applies to: 67-67, 74-78, 84-88
web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx (4)
17-22: Consider removing unused prop types.The
Propstype includessceneId,projectId, andworkspaceId, but these are not used in the component. Consider removing them if they are not needed for future implementations.If these props are intended for future use, consider adding a TODO comment explaining their purpose.
44-44: Consider handling potential undefined properties.The destructuring of
meDataassumes thatnameandYou could update the line as follows:
const { name = '', email = '' } = meData ?? {};
48-53: Clean up commented code in Navbar props.There are commented out props in the Navbar component. If these are not needed, consider removing them entirely. If they are intended for future use, add a TODO comment explaining their purpose.
Clean up the Navbar props:
<Navbar - // projectId={projectId} workspaceId={meData.myTeam?.id} - // sceneId={sceneId} page="settings" />
1-138: Overall, good implementation with minor improvements needed.The
AccountAndWorkSpaceSettingcomponent is well-structured and implements the required functionality for account and workspace settings. Here are some additional suggestions for improvement:
- Consider adding error boundaries to handle potential runtime errors gracefully.
- Implement proper error handling for the
handleUpdateUserPasswordfunction, possibly showing error messages to the user.- Add unit tests to ensure the component behaves correctly under different scenarios.
- Consider implementing lazy loading for the
AccountSettingcomponent to improve initial load time.To improve the overall architecture:
- Consider extracting the tab configuration into a separate file for better maintainability.
- Implement a more robust state management solution (e.g., React Context or Redux) if the application grows in complexity.
- Use a custom hook for handling the tab state and navigation to keep the main component cleaner.
Great job on implementing this feature! These suggestions will help improve the code quality and maintainability.
web/src/beta/lib/reearth-ui/components/TextInput/index.tsx (2)
26-26: LGTM! Consider adding JSDoc comment for better documentation.The addition of the
typeproperty enhances the flexibility of theTextInputcomponent, allowing it to be used for various input types. This is a good improvement.Consider adding a JSDoc comment to provide more information about the
typeproperty. For example:/** The type of the input (e.g., "text", "password", "email"). Defaults to "text" if not specified. */ type?: string;
95-95: LGTM! Consider setting a default value for thetypeprop.The
typeprop is correctly passed to theStyledInputcomponent, allowing dynamic setting of the input type. This implementation is correct and aligns with the component's enhanced flexibility.Consider setting a default value for the
typeprop to ensure consistent behavior when it's not explicitly provided. You can do this in the component's props destructuring:export const TextInput: FC<TextInputProps> = ({ // ... other props type = "text" }) => { // ... component logic }This ensures that the input defaults to a text input when no type is specified, maintaining backwards compatibility and preventing potential issues with undefined types.
web/src/services/api/meApi.ts (2)
73-73: Fix grammatical error in success messageThe success notification message says "Successfully delete user!" which is grammatically incorrect. It should be "Successfully deleted user!" to reflect the correct tense.
Apply this diff to fix the grammatical error:
-text: t("Successfully delete user!") +text: t("Successfully deleted user!")
64-64: Ensure consistent use of singular form in console log messageThe console log statement says "Failed to delete users", but the operation deletes a single user. For consistency, change "users" to "user".
Apply this diff to correct the message:
-console.log("GraphQL: Failed to delete users", errors); +console.log("GraphQL: Failed to delete user", errors);web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx (1)
118-119: RenameisMatchPasswordfor better readabilityThe variable
isMatchPasswordis true when passwords do not match, which might be confusing. Consider renaming it topasswordsMismatchor inverting the condition to improve clarity.Apply this diff to rename the variable:
- const isMatchPassword = - password !== passwordConfirmation && passwordConfirmation; + const passwordsMismatch = + passwordConfirmation && password !== passwordConfirmation;And update its usage accordingly:
- {isMatchPassword ? ( + {passwordsMismatch ? (
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (12)
- web/src/beta/features/AccountAndWorkSpaceSetting/hooks.ts (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx (1 hunks)
- web/src/beta/features/Dashboard/LeftSidePanel/profile.tsx (3 hunks)
- web/src/beta/features/Navbar/LeftSection/index.tsx (3 hunks)
- web/src/beta/lib/reearth-ui/components/IconButton/index.tsx (3 hunks)
- web/src/beta/lib/reearth-ui/components/TextInput/index.tsx (3 hunks)
- web/src/beta/pages/AccountSettingsPage/index.tsx (1 hunks)
- web/src/services/api/meApi.ts (2 hunks)
- web/src/services/routing/index.tsx (2 hunks)
🔇 Additional comments not posted (30)
web/src/beta/pages/AccountSettingsPage/index.tsx (4)
1-4: LGTM: Imports are appropriate and well-structured.The imports are correctly defined and align with the component's requirements. The use of relative imports for custom components (AccountAndWorkSpaceSetting and Page) and the import of FC from React are appropriate.
6-6: LGTM: Component definition is clear and concise.The AccountSettingPage component is correctly defined as a functional component using the FC type. The naming is consistent with the file name and its purpose.
14-14: LGTM: Export statement is correct.The default export of AccountSettingPage is appropriate and follows common React component export patterns.
1-14: Overall implementation looks good, verify password change functionality.The refactoring of this component significantly enhances the account page functionality, aligning well with the PR objective. The use of AccountAndWorkSpaceSetting suggests that the password change feature is implemented there.
To ensure the password change functionality is correctly implemented, please verify:
- The AccountAndWorkSpaceSetting component includes the password change interface.
- The necessary API calls for password change are implemented and working correctly.
You can use the following script to check for password-related functionality in the AccountAndWorkSpaceSetting component:
This will help confirm that the password change feature is present in the expected location.
✅ Verification successful
Password change functionality verified successfully.
The
AccountAndWorkSpaceSettingcomponent includes all necessary password change features as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for password-related functionality in AccountAndWorkSpaceSetting # Test: Search for password-related terms in the AccountAndWorkSpaceSetting component rg --type typescript -i 'password|changepassword' src/beta/features/AccountAndWorkSpaceSettingLength of output: 131
Script:
#!/bin/bash # Description: Check for password-related functionality in AccountAndWorkSpaceSetting # Test: Search for password-related terms in TypeScript files within the AccountAndWorkSpaceSetting component rg -i 'password|changepassword' src/beta/features/AccountAndWorkSpaceSetting --glob "*.ts" --glob "*.tsx"Length of output: 192
Script:
#!/bin/bash # Description: Locate the AccountAndWorkSpaceSetting component in the codebase # Test: Search for directories matching 'AccountAndWorkSpaceSetting' (case-insensitive) fd -i 'AccountAndWorkSpaceSetting' --type dLength of output: 93
Script:
#!/bin/bash # Description: Check for password-related functionality in AccountAndWorkSpaceSetting # Test: Search for password-related terms in TypeScript files within the AccountAndWorkSpaceSetting component rg -i 'password|changepassword' web/src/beta/features/AccountAndWorkSpaceSetting --glob "*.ts" --glob "*.tsx"Length of output: 14024
web/src/beta/features/AccountAndWorkSpaceSetting/hooks.ts (4)
1-2: LGTM: Imports are appropriate and concise.The imports are relevant to the hook's functionality, importing the necessary API service and React hook.
4-7: LGTM: Clear and appropriate type definition.The
UpdatePasswordTypeis well-defined and exported, which allows for type checking and potential reuse in other components.
9-34: LGTM: Well-structured custom hook with proper use of React patterns.The hook is well-organized, uses memoization for optimization, and follows React best practices. The returned object provides a clean interface for consumers of this hook.
23-26: Clarify the inclusion ofhandleDeleteUserfunction.The PR objectives didn't mention implementing account deletion functionality. Could you clarify why this function is included in the current PR? If it's not intended for use yet, consider removing it to keep the PR focused on the stated objectives.
web/src/services/routing/index.tsx (2)
1-1: LGTM: New import for AccountSettingPageThe import statement for the new AccountSettingPage component is correctly placed and follows the existing import style. This aligns with the PR objective of adding an account settings page.
Line range hint
1-41: Overall, the changes look good and align with the PR objectivesThe addition of the new route for the account settings page is well-implemented and follows existing patterns in the file. The changes are minimal and focused, which is appreciated.
Please ensure that appropriate error handling and access control are implemented for this new route, either in the AccountSettingPage component or in a higher-level component. You may want to verify:
- That unauthenticated users are redirected to a login page when trying to access this route.
- That any errors that occur while loading or rendering the AccountSettingPage are properly caught and displayed to the user.
You can use the following script to check for basic error handling and authentication checks in the AccountSettingPage component:
If these checks don't yield results, consider implementing them to ensure a robust user experience.
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx (7)
1-3: LGTM: Imports are appropriate and well-organized.The import statements are concise and import only the necessary components and functions. The use of a custom UI library and internationalization service indicates a well-structured project.
5-15: LGTM: InnerPage component is well-designed and flexible.The InnerPage styled component is well-implemented with the following strengths:
- Flexible options for width and transparency through props.
- Consistent use of theme values for styling, ensuring design system compliance.
- Responsive design with max-width settings.
25-33: LGTM: SettingsWrapper component is well-structured.The SettingsWrapper styled component is well-implemented:
- Utilizes flexbox for efficient layout management.
- Applies consistent styling to child elements using nested selectors.
- Makes good use of theme values for border colors.
35-39: LGTM: SettingsFields component is concise and effective.The SettingsFields styled component is well-designed:
- Uses flexbox with column direction for vertical stacking of fields.
- Applies consistent spacing between items using theme values.
41-46: LGTM: SettingsRow component provides flexible row layouts.The SettingsRow styled component is well-implemented:
- Utilizes flexbox with space-between justification for flexible layouts.
- Applies consistent spacing between items using theme values.
62-66: LGTM: ButtonWrapper component is concise and effective.The ButtonWrapper styled component is well-implemented:
- Uses flexbox with end justification for aligning buttons to the right.
- Applies consistent spacing between buttons using theme values.
1-79: Overall, the file is well-structured with room for minor improvements.The
common.tsxfile provides a solid foundation for the account and workspace settings UI:
- Styled components are well-organized and make good use of the theme for consistent styling.
- The functional component (ArchivedSettingNotice) demonstrates good use of UI components and internationalization.
Areas for potential improvement:
- Enhance responsiveness of some components (e.g., InnerSidebar).
- Increase reusability of certain components by accepting props (e.g., ArchivedSettingNotice).
- Consider simplifying or enhancing very basic components (e.g., SettingsRowItem).
- Improve accessibility for the Thumbnail component.
These suggestions aim to make the components more flexible and maintainable as the project grows.
web/src/beta/features/Navbar/LeftSection/index.tsx (2)
4-5: LGTM: Import statements updated correctly.The new imports for
TypographyanduseThemeare consistent with the changes made in the component.Also applies to: 8-8
36-36: LGTM: Theme hook added correctly.The
useThemehook is properly implemented to access the theme object, which is used later in the component.web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx (2)
27-34: LGTM: Component definition and state initialization.The component definition, use of the
useThook for internationalization, and state initialization for the password modal visibility are well-implemented.
91-101: LGTM: Styled components are well-defined.The
PasswordWrapperandPasswordInputWrapperstyled components are well-implemented. They use appropriate CSS properties and leverage the theme for consistent spacing.web/src/beta/features/Dashboard/LeftSidePanel/profile.tsx (3)
11-11: LGTM: Import statement for navigation hook.The addition of the
useNavigatehook fromreact-router-domis appropriate for implementing the new navigation functionality to the account settings page.
41-41: LGTM: Proper initialization of navigation hook.The
useNavigatehook is correctly initialized at the component level, following React hooks usage rules.
Line range hint
1-145: Summary: Implementation of Account Settings navigation looks good.The changes to add the Account Settings menu item and its associated navigation are well-implemented and consistent with the existing codebase. The code follows React best practices and maintains the current structure of the
Profilecomponent.A few final considerations:
- Ensure that the "/settings/account" route is properly set up in your routing configuration.
- Consider adding a unit test for the new navigation functionality.
- Update any relevant documentation to reflect this new feature.
Great job on implementing this feature!
web/src/beta/lib/reearth-ui/components/IconButton/index.tsx (4)
28-29: LGTM! Prop destructuring updated correctly.The
hasBorderprop has been correctly added to the destructured props in theIconButtoncomponent. This change is consistent with theIconButtonPropstype update and allows the prop to be passed down to theStyledButtoncomponent.
49-49: LGTM! StyledButton props updated correctly.The
hasBorderprop is correctly passed to theStyledButtoncomponent. This change enables the styled component to apply conditional styling based on thehasBordervalue, which is consistent with the component's enhanced customization capabilities.
57-57: LGTM! StyledButton type definition updated correctly.The
StyledButtontype definition has been properly updated to include:
- The new "medium" size option, maintaining consistency with
IconButtonProps.- The
hasBorderproperty, ensuring proper typing for the styled component.These changes align well with the modifications made to the
IconButtonPropstype and the component's enhanced functionality.Also applies to: 61-61
Line range hint
1-145: Overall, great implementation! A few final suggestions.The changes to the
IconButtoncomponent successfully implement the new "medium" size option and thehasBorderprop. The modifications are consistent across the component's type definitions, props, and styled component implementation.To further improve the code:
- Update the component's documentation to reflect the new size option and
hasBorderprop.- Consider the suggested refactoring for size calculations to improve maintainability.
- Add unit tests to cover the new functionality, especially the "medium" size and
hasBorderprop behavior.Great job on enhancing the component's flexibility and customization options!
web/src/beta/lib/reearth-ui/components/TextInput/index.tsx (2)
42-43: LGTM! Consistent with the interface update.The addition of the
typeparameter to the component's function signature is correct and consistent with theTextInputPropsinterface update.
Line range hint
1-168: Overall implementation aligns well with PR objectives.The changes to the
TextInputcomponent, particularly the addition of thetypeprop, align well with the PR objective of adding an account page for changing passwords. This enhancement allows theTextInputcomponent to be used for password inputs, which is crucial for the new account settings page.A few minor suggestions have been made to improve documentation and default behavior, but overall, the implementation is sound and contributes effectively to the new feature.
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
ZTongci
changed the title
ZTongci
force-pushed
the
feat/user_setting
branch
from
8b21cd5 to
34aeb4d
Compare
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
Outdated
Show resolved
Hide resolved
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
Outdated
Show resolved
Hide resolved
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx
Outdated
Show resolved
Hide resolved
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
web/src/services/i18n/translations/en.yml (1)
8-20: LGTM: Comprehensive password-related translation keys.The new keys for password strength indicators, validation messages, and password change functionality are well-defined and user-friendly.
Consider adjusting the key on line 20 for consistency:
-'"repeatPassword" Passwords need to match': '' +repeatPassword_passwords_need_to_match: ''This change would align the key format with other keys in the file.
web/src/services/i18n/translations/ja.yml (3)
Line range hint
21-61: Add missing translations for project and asset management termsThe translations for project and asset management are generally good, with appropriate modifications. However, there are several missing translations that should be added:
- Line 53:
Select Asset: ''- Line 58:
Upload File: ''- Line 59:
Search in all assets library: ''- Line 60:
Assets selected: ''- Line 61:
Asset selected: ''Please provide appropriate Japanese translations for these terms to ensure a consistent user experience.
The existing translations are accurate and natural-sounding in Japanese. The replacement of the generic "Notice" translation with a more specific message about archived projects (lines 21-22) is a good improvement.
Line range hint
62-180: Add missing translations for various UI elementsThe translations for various UI elements are generally good and accurate. However, there are several missing translations that should be added:
- Line 181:
Visualizer: ''- Line 183:
Switch workspace: ''Please provide appropriate Japanese translations for these terms to ensure a consistent user experience.
The existing translations are accurate and natural-sounding in Japanese. It's good that some technical terms (e.g., "Plugin", "Vector Tile") are left untranslated, as this maintains clarity for users familiar with these terms.
Line range hint
181-343: Add missing translations for error messages and user feedbackThe translations for error messages and user feedback are generally good. However, there are critical missing translations that should be added, especially for account-related actions:
- Line 340:
Failed to update user password.: ''- Line 341:
Successfully updated user password!: ''- Line 342:
Failed to delete user.: ''- Line 343:
Successfully delete user!: ''Please provide appropriate Japanese translations for these messages to ensure users receive proper feedback for important account actions.
The existing translations for error messages and user feedback are accurate and natural-sounding in Japanese. They provide clear and helpful information to users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (14)
- web/src/beta/features/AccountAndWorkSpaceSetting/hooks.ts (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx (1 hunks)
- web/src/beta/features/Dashboard/LeftSidePanel/profile.tsx (3 hunks)
- web/src/beta/features/Navbar/LeftSection/index.tsx (3 hunks)
- web/src/beta/lib/reearth-ui/components/IconButton/index.tsx (3 hunks)
- web/src/beta/lib/reearth-ui/components/TextInput/index.tsx (3 hunks)
- web/src/beta/pages/AccountSettingsPage/index.tsx (1 hunks)
- web/src/services/api/meApi.ts (2 hunks)
- web/src/services/i18n/translations/en.yml (4 hunks)
- web/src/services/i18n/translations/ja.yml (4 hunks)
- web/src/services/routing/index.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- web/src/beta/features/AccountAndWorkSpaceSetting/hooks.ts
- web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx
- web/src/beta/features/Dashboard/LeftSidePanel/profile.tsx
- web/src/beta/features/Navbar/LeftSection/index.tsx
- web/src/beta/lib/reearth-ui/components/IconButton/index.tsx
- web/src/beta/lib/reearth-ui/components/TextInput/index.tsx
- web/src/beta/pages/AccountSettingsPage/index.tsx
- web/src/services/api/meApi.ts
- web/src/services/routing/index.tsx
🔇 Additional comments (4)
web/src/services/i18n/translations/en.yml (3)
4-7: LGTM: New account-related translation keys.The new keys for "Account", "Name", "Email address", and "Password" are well-defined and align with the PR objective of adding an account page.
Line range hint
1-343: Summary of translation file reviewThe changes to the
en.ymlfile generally align with the PR objectives of adding an account page with password change functionality. The new translation keys provide good coverage for the new features.However, there are a few areas that require attention:
- Inconsistent naming conventions for translation keys.
- Potential duplicate and unnecessary keys (e.g., "Notice" and user deletion messages).
- Possible missing translations for some account-related functionality.
Please address the issues raised in the previous comments to improve the overall quality and consistency of the translations. Once these are resolved, the translation file will be well-prepared to support the new account page functionality.
Line range hint
1-343: Review naming conventions and completeness.
There are inconsistencies in the naming conventions used for translation keys throughout the file. Consider adopting a consistent naming convention (e.g., snake_case) for all keys to improve maintainability.
To ensure all necessary translations for the new account page functionality are included, please review the following:
#!/bin/bash # Description: Check for completeness of account-related translations # Search for account-related keys echo "Account-related keys:" grep -iE 'account|password|email|name' web/src/services/i18n/translations/en.yml # Check for potentially missing keys echo "\nPotentially missing keys (common account-related terms not found):" for term in "username" "profile" "settings" "security" "privacy" "logout" "sign out" "delete account" "update profile" do if ! grep -qi "$term" web/src/services/i18n/translations/en.yml; then echo "- $term" fi donePlease review the output of this script to ensure all necessary translations for the account page are present.
web/src/services/i18n/translations/ja.yml (1)
Line range hint
1-343: Overall translation quality is good, but some key translations are missingThe Japanese translations provided in this file are generally accurate, natural-sounding, and appropriate for their context. The additions and modifications align well with the PR objectives of adding account-related functionality.
However, there are several missing translations throughout the file, particularly for new account-related features and some UI elements. These missing translations should be addressed to ensure a consistent and complete Japanese user experience.
Key areas to focus on:
- Account and password-related terms (lines 4-20)
- Project and asset management terms (lines 21-61)
- Various UI elements (lines 62-180)
- Error messages and user feedback, especially for account actions (lines 181-343)
Please review the previous comments for specific lines that need translations added.
The existing translations are of good quality and accurately convey the intended meanings in Japanese.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/common.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
...c/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
web/src/services/i18n/translations/ja.yml (1)
Line range hint
1-392: Overall improvement in translations, but some gaps remainThank you for adding many new translations to improve the Japanese localization of the application. The additions cover various aspects of the UI and enhance the user experience for Japanese users.
However, there are still some important translations missing, particularly for:
- Account-related terms (e.g., "Account", "Email address")
- Password change functionality
- Account settings
- User feedback messages for password updates and account deletion
To ensure a fully localized experience for Japanese users, please complete the missing translations highlighted in the previous comments.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx (1 hunks)
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx (1 hunks)
- web/src/services/i18n/translations/en.yml (4 hunks)
- web/src/services/i18n/translations/ja.yml (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/beta/features/AccountAndWorkSpaceSetting/index.tsx
- web/src/beta/features/AccountAndWorkSpaceSetting/innerPages/AccountSetting/PasswordModal/index.tsx
- web/src/services/i18n/translations/en.yml
🔇 Additional comments (2)
web/src/services/i18n/translations/ja.yml (2)
4-7:⚠️ Potential issueComplete missing account-related translations
Thank you for adding translations for "Name" and "Password". However, there are still two missing translations:
- Line 4:
Account: ''- Line 6:
Email address: ''Please provide appropriate Japanese translations for these terms to ensure a complete user experience.
15-18:⚠️ Potential issueAdd missing password change translations
Thank you for adding the translation for the password protection advice. However, there are still three missing translations:
- Line 15:
Change password: ''- Line 17:
New password: ''- Line 18:
New password (for confirmation): ''Please provide appropriate Japanese translations for these terms to ensure a complete user experience for the password change functionality.
Co-authored-by: tcsola <tcsola@tcsolaMacBook-Pro.local> Co-authored-by: lby <icesunex@hotmail.com>
Overview
What I've done
Adding page for account setting
Using hook to show userName and userEmail for account setting page
Adding interface to update account password
What I haven't done
add function to delete account
How I tested
Using my account, click change password. And using new password login successful.
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Style
Localization