Enhance error handling in user settings routes with user-facing error mapping #1618
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| */ | ||
| function AccountSettingsFallback(props: NimbusErrorBoundaryFallbackProps) { | ||
| const { | ||
| title = "Account settings failed to load", |
There was a problem hiding this comment.
Can this page even fail to load when it has no loader? I guess the error boundary can catch errors from the actions, but for those errors this error message is a bit off?
| function AccountSettingsFallback(props: NimbusErrorBoundaryFallbackProps) { | ||
| const { | ||
| title = "Account settings failed to load", | ||
| description = "Reload the account tab or return to settings.", |
There was a problem hiding this comment.
Is the "return to settings" part helpful to the user if the error view doesn't provide a way to do so? If my assumption that the error message is shown only as a reaction to error when submitting an action, would something like "Reload the page to try again" be more suitable?
| const { | ||
| title = "Account settings failed to load", | ||
| description = "Reload the account tab or return to settings.", | ||
| retryLabel = "Reload", |
There was a problem hiding this comment.
Nitpick: Haven't checked the rest of the PRs but I assume "Reload" will be quite common label, so it should probably be the default value instead of the current "Retry"? Would save us some boilerplate.
| */ | ||
| function UserSettingsFallback(props: NimbusErrorBoundaryFallbackProps) { | ||
| const { | ||
| title = "Settings failed to load", |
There was a problem hiding this comment.
Can this page fail to load? This time there doesn't seem to be any actions either. Do we want to wrap all the things just in case?
| function UserSettingsFallback(props: NimbusErrorBoundaryFallbackProps) { | ||
| const { | ||
| title = "Settings failed to load", | ||
| description = "Reload the settings page or return to the dashboard.", |
Oksamies
changed the base branch from
11-19-_ts-api-react-actions_enhance_error_handling_in_api_actions_and_hooks_with_user-facing_error_mapping
to
graphite-base/1618
Oksamies
force-pushed
the
11-19-enhance_error_handling_in_user_settings_routes_with_user-facing_error_mapping
branch
from
02b2112 to
cca84a1
Compare
Oksamies
force-pushed
the
graphite-base/1618
branch
from
cd40580 to
5ac0d36
Compare
Oksamies
changed the base branch from
graphite-base/1618
to
11-19-_ts-api-react-actions_enhance_error_handling_in_api_actions_and_hooks_with_user-facing_error_mapping
Oksamies
changed the base branch from
11-19-_ts-api-react-actions_enhance_error_handling_in_api_actions_and_hooks_with_user-facing_error_mapping
to
graphite-base/1618
Oksamies
force-pushed
the
11-19-enhance_error_handling_in_user_settings_routes_with_user-facing_error_mapping
branch
from
cca84a1 to
9f14641
Compare
Oksamies
force-pushed
the
graphite-base/1618
branch
from
5ac0d36 to
de2572f
Compare
Oksamies
changed the base branch from
graphite-base/1618
to
11-19-_ts-api-react-actions_enhance_error_handling_in_api_actions_and_hooks_with_user-facing_error_mapping
No description provided.