-
Notifications
You must be signed in to change notification settings - Fork 44
refactor(settings): improve responsive styles on small viewports #636
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
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
samui-wallet-api | 329a2ac | Commit Preview URL Branch Preview URL |
Dec 01 2025, 07:41 AM |
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.
Important
Looks good to me! 👍
Reviewed everything up to 84f2a0d in 1 minute and 10 seconds. Click for details.
- Reviewed
497lines of code in20files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/i18n/locales/en/settings.json:42
- Draft comment:
Improved phrasing in networkInputTypeDescription enhances clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/i18n/src/resources.d.ts:106
- Draft comment:
Consistent text updates in resource definitions; no action required. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/settings/src/settings-feature-wallet-add-account.tsx:35
- Draft comment:
Consider using 'e.message' (or a more informative error format) in catch blocks for better error clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/settings/src/settings-feature-wallet-list.tsx:22
- Draft comment:
Renaming the 'active' prop to 'activeId' improves clarity. Verify that useSetting returns the expected type. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/settings/src/ui/settings-ui-network-form-create.tsx:30
- Draft comment:
Using responsive spacing classes (e.g. 'space-y-2 md:space-y-6') is a nice improvement for small viewports. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. packages/settings/src/ui/settings-ui-wallet-list-item.tsx:20
- Draft comment:
Using useLocation() to capture 'from' and passing it via Link state improves navigation context. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. packages/settings/src/ui/settings-ui-wallet-form-import.tsx:87
- Draft comment:
The Switch for redirect is well implemented; consider adding an aria-label for improved accessibility. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. packages/settings/src/ui/settings-ui-network-list-item.tsx:37
- Draft comment:
UiConfirm usage is appropriate. Double-check deletion confirmation text consistency across similar components. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_nWNlpKsS3SENmuXc
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
84f2a0d to
3532ada
Compare
3532ada to
826ad5d
Compare
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.
Caution
Changes requested ❌
Reviewed 826ad5d in 2 minutes and 19 seconds. Click for details.
- Reviewed
521lines of code in21files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/settings/src/ui/settings-ui-wallet-list-item.tsx:38
- Draft comment:
Use the translation function (t) for confirmation texts in UiConfirm instead of hardcoded strings to ensure consistency and localization. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. packages/settings/src/ui/settings-ui-wallet-form-generate.tsx:41
- Draft comment:
You can simplify the onSubmit handler by passing 'submit' directly to form.handleSubmit, e.g., onSubmit={form.handleSubmit(submit)}. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/settings/src/ui/settings-ui-wallet-form-update.tsx:34
- Draft comment:
Consider passing 'submit' directly to form.handleSubmit to simplify the onSubmit handler, e.g., onSubmit={form.handleSubmit(submit)}. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. packages/settings/src/settings-feature-wallet-details.tsx:47
- Draft comment:
The button's title uses the translation keyactionEditWalletMessage, but the displayed text usesactionAddAccount. This inconsistency might be a typo. Please verify which text is intended. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_Wl8raeCVNILjug0Y
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
826ad5d to
329a2ac
Compare
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.
Important
Looks good to me! 👍
Reviewed 329a2ac in 1 minute and 5 seconds. Click for details.
- Reviewed
549lines of code in22files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/settings/src/settings-feature-general-danger-delete-database.tsx:36
- Draft comment:
Notice thatnavigate('/')is being awaited even thoughuseNavigatereturns void. It isn’t necessary to await navigation. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. packages/settings/src/settings-feature-wallet-generate.tsx:29
- Draft comment:
Consider using async/await instead of mixing promise chaining with .then for consistency. For example, await the mutation then call navigate. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/shell/src/ui/shell-ui-layout.tsx:22
- Draft comment:
Returning null when there are no accounts might leave the UI blank. Consider displaying a placeholder or loader for better UX. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/settings/src/settings-feature-wallet-add-account.tsx:45
- Draft comment:
Error messages (e.g. in catch blocks) are used directly without localization. Consider using the translation function for consistency. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
5. packages/settings/src/ui/settings-ui-network-form-create.tsx:30
- Draft comment:
The usage of inline arrow functions in render props (e.g., for ToggleGroupItem) is acceptable; just ensure that such functions do not cause unnecessary rerenders if performance becomes a concern. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
Workflow ID: wflow_GHO8DBzWCjdoAZlf
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
tobeycodes
left a comment
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.
LGTM
Description
This PR makes the settings screen more consistent on smaller viewports
Fixes #641
Important
Refactor settings screen for improved responsive styles on small viewports and correct minor text issues in i18n files.
space-y-6tospace-y-2 md:space-y-6insettings-feature-general-danger-delete-database.tsx,settings-feature-general.tsx,settings-feature-wallet-add-account.tsx,settings-feature-wallet-details.tsx,settings-feature-wallet-generate.tsx,settings-feature-wallet-list.tsx,settings-feature-wallet-update.tsx,settings-ui-network-form-create.tsx,settings-ui-network-form-update.tsx,settings-ui-wallet-create-options.tsx,settings-ui-wallet-form-generate.tsx,settings-ui-wallet-form-import.tsx,settings-ui-wallet-form-update.tsx.shell-ui-layout.tsxto change header and footer background opacity frombg-secondary/50tobg-secondary/30.settings.jsonandresources.d.ts.settings.jsonandresources.d.ts.SettingsUiNetworkItemandSettingsUiNetworkListItemcomponents for network item display.SettingsUiWalletListItemto includeuseLocationfor navigation state.This description was created by
for 329a2ac. You can customize this summary. It will automatically update as commits are pushed.