-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Settings revamp #5745
base: main
Are you sure you want to change the base?
Settings revamp #5745
Conversation
|
4ffe292
to
2a0d8b3
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.
I noticed a few strings that could be marked for translation :)
Thanks @surfdude29, forgot to mark them after copy/pasting out of the storybook :) |
This looks really, really good, so much better than it is now, excellent work! 👌 I have one piece of feedback and one suggestion. I'm not sure it's ideal to have the Accessibility and Appearance screens needing to be accessed via a sub-page rather than directly from the Settings screen. To me, they both seem important enough that the user should be able to access them directly. I realise that it would add one more item to the main Settings screen and thus more scrolling would be necessary to see all the options, but my two cents are that it would be a worthwhile tradeoff. The in-app browser pref would fit nicely in Content and Media imo. And my suggestion is to consider adding links to the Community Guidelines page and to the Copyright Policy page on the About screen. This would not only make it more convenient to access these pages directly from the app, but it would also mean that the five Support screens could then be removed: Lines 38 to 42 in 4c3c10d
|
thanks @surfdude29! Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
Thanks @surfdude29! Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
8d4ba3a
to
817154b
Compare
onChange={value => setHapticsDisabled(value)} | ||
style={[a.w_full]}> | ||
<Toggle.LabelText style={[a.flex_1]}> | ||
<Trans>Disable haptic feedback</Trans> |
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.
Why not set it to on by default and disabled when turned off? The same goes for the Autoplay setting. (#4274)
Gated behind
new_settings
Screens:
Dialogs:
Other:
<Select>
component for languages screen