-
Notifications
You must be signed in to change notification settings - Fork 27
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(ui): update reset modal UX x wording #1564
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request adds new reset-related keys to multiple locale JSON files. The new keys provide detailed explanations for various reset options using different configurations, including retaining or removing wallet information. In addition, UI components have been updated to incorporate enhanced styling properties (such as a new radio button style type) and refactored reset flow by moving dialog management into a dedicated component. The overall change improves modularity and clarity in both localization and component behavior for resetting settings. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RSB as ResetSettingsButton
participant RSD as ResetSettingsDialog
participant BE as Backend
U->>RSB: Click "Reset" button
RSB->>RSD: Open reset dialog
U->>RSD: Choose reset option & confirm
RSD->>BE: Call reset_settings API with selected option
BE-->>RSD: Return success/failure
RSD->>U: Close dialog and update UI state
Suggested labels
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
|
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: 0
🧹 Nitpick comments (8)
public/locales/id/settings.json (1)
100-104
: Translation Consistency: Ensure full localization of reset keys.
In the Indonesian locale file, several new reset-related keys have been added. Notice that while the key"reset-permanently"
is properly localized into Indonesian, keys such as"reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
remain in English. To maintain a consistent user experience and proper localization, please either provide the Indonesian translations for these keys or add a TODO note indicating that translations are pending.public/locales/ko/settings.json (1)
100-104
: Localization Review: Align reset key translations in Korean.
In this file, while"reset-permanently"
is provided in Korean, the keys"reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
remain in English. Please verify if this is intentional (e.g., awaiting proper translations) or update these entries to use appropriate Korean text for better consistency in the user interface.public/locales/de/settings.json (1)
100-104
: Inconsistent Translations in German Locale.
The new reset-related keys show mixed language content. Although"reset-permanently"
is translated into German, keys like"reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
are still in English. Please update these keys with proper German translations or add comments/TODOs if the translations are expected to be provided later.public/locales/ru/settings.json (1)
100-104
: Review Translation Consistency in Russian Locale.
Similar to other locale files, the Russian file shows mixed language usage:"reset-permanently"
is correctly localized, whereas"reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
remain in English. For consistency and a seamless user experience, please either supply the Russian translations for these keys or document that they are pending translation.public/locales/pl/settings.json (1)
102-104
: Translate the new reset-related strings to Polish.The newly added reset functionality strings are still in English, while the rest of the file contains Polish translations. These need to be translated to maintain consistency with the rest of the interface.
For example:
- "reset": "Reset", + "reset": "Resetuj",For the longer explanatory texts, consider working with a translator to ensure accurate Polish translations that maintain the meaning of the original text.
Also applies to: 106-106
public/locales/tr/settings.json (1)
100-102
: Translate the new reset-related strings to Turkish.The newly added strings for the reset functionality are in English, while the rest of the file contains Turkish translations. These should be translated to Turkish to maintain UI language consistency.
For example:
- "reset": "Reset", + "reset": "Sıfırla",For the longer explanatory texts, work with a translator to ensure accurate Turkish translations that preserve the meaning of the English text.
Also applies to: 104-104
src/containers/floating/Settings/sections/general/ResetSettingsDialog.tsx (2)
92-104
: Consider adding an empty state message when no option is selected.Currently when no option is selected, the explainer area is completely empty. Consider displaying a prompt message like "Please select an option above" to guide the user.
{!selectedItem ? ( - '' + t('reset-select-option', 'Please select an option above') ) : (
96-101
: Use explicit context names for clarity in translation files.The context parameter used here (
context={selectedItem === 'config_and_wallet' ? 'wallet' : ''}
) makes the translation somewhat implicit. Consider using more descriptive context names that clearly indicate their purpose to translators.<Trans ns="settings" i18nKey="reset-config-explainer" components={{ strong: <strong /> }} - context={selectedItem === 'config_and_wallet' ? 'wallet' : ''} + context={selectedItem === 'config_and_wallet' ? 'wallet_reset' : 'config_reset'} />
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
public/locales/af/settings.json
(1 hunks)public/locales/cn/settings.json
(1 hunks)public/locales/de/settings.json
(1 hunks)public/locales/en/settings.json
(1 hunks)public/locales/fr/settings.json
(1 hunks)public/locales/hi/settings.json
(1 hunks)public/locales/id/settings.json
(1 hunks)public/locales/ja/settings.json
(1 hunks)public/locales/ko/settings.json
(1 hunks)public/locales/pl/settings.json
(1 hunks)public/locales/ru/settings.json
(1 hunks)public/locales/tr/settings.json
(1 hunks)src/components/elements/buttons/BaseButton.styles.ts
(1 hunks)src/components/elements/inputs/RadioButton.styles.ts
(4 hunks)src/components/elements/inputs/RadioButton.tsx
(1 hunks)src/containers/floating/Settings/sections/general/ResetSettingsButton.tsx
(2 hunks)src/containers/floating/Settings/sections/general/ResetSettingsDialog.tsx
(1 hunks)src/i18initializer.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Check title
- GitHub Check: Check i18n
- GitHub Check: tauri-build
🔇 Additional comments (21)
src/i18initializer.ts (1)
134-134
: Good improvement to i18n configuration.Adding a contextSeparator helps create better structured translation keys, which is particularly useful for the new reset-related translations mentioned in the PR.
src/components/elements/buttons/BaseButton.styles.ts (2)
22-22
: Prevent text selection on buttons.Adding
-webkit-user-select: none;
prevents text selection in webkit browsers, improving the user experience by avoiding accidental text selection during clicks.
28-28
: Fix disabled button hover behavior.Adding
pointer-events: none;
ensures that disabled buttons don't show hover effects, addressing the issue mentioned in the PR objectives.src/components/elements/inputs/RadioButton.tsx (2)
7-7
: Excellent enhancement to RadioButton component with new minimal style option.Adding the 'minimal' style type increases the component's flexibility, which is exactly what's needed for the improved Reset Settings dialog mentioned in the PR objectives.
Also applies to: 12-12, 14-14
16-16
: Properly passing style type to styled components.Correctly forwarding the styleType prop to the styled components ensures consistent styling throughout the component hierarchy.
Also applies to: 22-22
src/components/elements/inputs/RadioButton.styles.ts (5)
2-3
: Good imports and interface update for new styling.Properly imported the RadioType and utility function needed for the new styling options.
Also applies to: 7-7
27-45
: Well implemented background styling for minimal variant.Using convertHexToRGBA for the minimal style creates a subtler background appearance that's appropriate for the reset dialog context.
🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
50-59
: Good size differentiation between style variants.The minimal variant's reduced padding and height creates a more compact UI element that's well-suited for the reset dialog options.
65-83
: Improved text styling for minimal variant.Using accent text color for the minimal style helps distinguish it from the primary style and creates visual hierarchy.
🧰 Tools
🪛 Biome (1.9.4)
[error] 78-78: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
88-96
: Appropriate text alignment for different use cases.Left-aligned text for minimal style works well for option lists, while centered and capitalized text suits the primary style's more prominent appearance.
public/locales/en/settings.json (1)
100-109
: Reset Keys Localization: Approved for the English locale.
All new reset-related keys are provided in clear, consistent English that aligns well with the PR’s UX updates. No changes are needed.public/locales/fr/settings.json (1)
100-104
: Inconsistent Translation for Reset Functionality Keys
The newly added reset-related keys (i.e."reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
) are still in English, whereas the rest of the file is in French. Please verify whether these entries should be translated to French or if they are intentionally left in English (for example, if English is used as a fallback language in this context).public/locales/hi/settings.json (1)
100-104
: Mixed Language in Reset Entries for Hindi Locale
In this file the new keys for reset functionality are partially inconsistent: while"reset-permanently"
is in Hindi, the other keys ("reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
) remain in English. Please confirm if these keys are intended to be left untranslated or if they should be fully localized in Hindi.public/locales/cn/settings.json (1)
100-104
: Reset Functionality Keys Not Localized in Chinese
The additions for reset functionality are appearing with English text ("reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
) in a file that otherwise contains Chinese translations. Please verify if these keys should be translated into Chinese to maintain language consistency.public/locales/af/settings.json (1)
100-104
: Mixed Language Usage for Reset Keys in Afrikaans Translation
The new reset-related keys are added with English text (e.g.,"reset": "Reset"
, etc.), while the rest of the file is in Afrikaans. Confirm whether these entries should be translated to Afrikaans or are purposefully kept in English.public/locales/ja/settings.json (1)
100-104
: Reset Functionality Keys Remain in English in the Japanese Localization
In this Japanese translation file, the newly added keys for reset functionality (including"reset"
,"reset-config-explainer"
,"reset-config-explainer-wallet"
, and"reset-restart"
) are in English rather than Japanese. Please verify if these values should be localized for consistency with the rest of the file.src/containers/floating/Settings/sections/general/ResetSettingsDialog.tsx (3)
43-48
: LGTM: Good state management for dialog interaction.The component properly initializes state variables and has a clean reset mechanism when the dialog is closed.
49-62
: LGTM: Well-structured reset function with proper error handling.The
resetSettings
function correctly:
- Sets loading state
- Invokes the backend function with appropriate parameters
- Handles success and error cases
- Cleans up state appropriately in both scenarios
113-120
: LGTM: Button disabled state prevents accidental resets.Good implementation of disabling the Reset button until an option is selected, preventing accidental resets without clear user intent.
src/containers/floating/Settings/sections/general/ResetSettingsButton.tsx (2)
1-12
: LGTM: Clean import statements with reduced dependencies.The refactored component now has minimal imports, focusing only on what's needed for its reduced responsibilities.
14-35
: LGTM: Well-refactored component with improved separation of concerns.The component has been effectively simplified by delegating dialog functionality to the
ResetSettingsDialog
component. This refactoring:
- Improves readability and maintainability
- Follows the single responsibility principle
- Makes the component easier to test
This is a good example of component composition in React.
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: 0
🧹 Nitpick comments (4)
public/locales/id/settings.json (4)
101-101
: Localization Consistency: Verify the 'reset' label.
The new key"reset"
has the value"Reset"
. Since the rest of this locale file is in Indonesian, please confirm whether the term should be localized (for example,"Atur Ulang"
) or if using English here is intentional.
102-102
: Localization Language Check: 'reset-config-explainer'.
The string for"reset-config-explainer"
is currently in English:"This option removes all configuration, settings, databases, logs, peers, etc., but will keep the wallet database so you retain your existing wallet address and balance."
This is inconsistent with the Indonesian translations in other parts of the file. If an Indonesian translation is required, consider updating it. For example, one possible translation might be:
"Opsi ini akan menghapus semua konfigurasi, pengaturan, basis data, log, rekan, dll., tetapi akan mempertahankan basis data dompet sehingga Anda <strong>mempertahankan alamat dan saldo dompet Anda saat ini</strong>."
103-103
: Localization Language Check: 'reset-config-explainer-wallet'.
The value provided for"reset-config-explainer-wallet"
is in English:"This option will remove all configuration, settings, databases, logs, peer connections, etc. for everything in Universe, including your existing wallet address and balance."
To maintain consistency with the Indonesian locale, please verify if this should be translated. A possible translation could be:
"Opsi ini akan menghapus semua konfigurasi, pengaturan, basis data, log, sambungan rekan, dll. untuk seluruh Universe, <strong>termasuk alamat dan saldo dompet Anda saat ini</strong>."
105-105
: Localization Language Mismatch: 'reset-restart'.
The string for"reset-restart"
is in English:"Tari Universe will restart automatically once the reset is complete"
whereas the surrounding content is predominantly in Indonesian (for example,
"reset-permanently"
is in Indonesian). Please ensure the language remains consistent. A possible Indonesian version might be:
"Tari Universe akan mulai ulang secara otomatis setelah pengaturan ulang selesai"
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locales/af/settings.json
(1 hunks)public/locales/cn/settings.json
(1 hunks)public/locales/de/settings.json
(1 hunks)public/locales/en/settings.json
(1 hunks)public/locales/fr/settings.json
(1 hunks)public/locales/hi/settings.json
(1 hunks)public/locales/id/settings.json
(1 hunks)public/locales/ja/settings.json
(1 hunks)public/locales/ko/settings.json
(1 hunks)public/locales/pl/settings.json
(1 hunks)public/locales/ru/settings.json
(1 hunks)public/locales/tr/settings.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- public/locales/cn/settings.json
- public/locales/af/settings.json
- public/locales/ru/settings.json
- public/locales/tr/settings.json
- public/locales/de/settings.json
- public/locales/en/settings.json
- public/locales/ja/settings.json
- public/locales/hi/settings.json
- public/locales/ko/settings.json
- public/locales/fr/settings.json
- public/locales/pl/settings.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tauri-build
Request from @metalaureate: "Reset all configuration settings, but leave wallet intact" |
Description
Motivation and Context
How Has This Been Tested?
reset settings only:
Screen.Recording.2025-02-25.at.14.58.23.mov
reset wallet:
Screen.Recording.2025-02-25.at.15.02.31.mov
screenshots:
Summary by CodeRabbit
New Features
Style
Refactor