Skip to content
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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Settings revamp #5745

wants to merge 31 commits into from

Conversation

mozzius
Copy link
Member

@mozzius mozzius commented Oct 14, 2024

Gated behind new_settings

Screens:

Dialogs:

Other:

  • New <Select> component for languages screen
Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 50 42 Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 50 49 Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 50 56 Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 51 06
Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 51 19 Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 51 26 Simulator Screenshot - iPhone 16 - 2024-10-15 at 14 51 42

Copy link

github-actions bot commented Oct 14, 2024

Old size New size Diff
7.96 MB 7.96 MB 0 B (0.00%)

Copy link
Contributor

@surfdude29 surfdude29 left a 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 :)

src/screens/Settings/AccountSettings.tsx Outdated Show resolved Hide resolved
src/screens/Settings/AccountSettings.tsx Outdated Show resolved Hide resolved
src/screens/Settings/AccountSettings.tsx Outdated Show resolved Hide resolved
@mozzius
Copy link
Member Author

mozzius commented Oct 15, 2024

Thanks @surfdude29, forgot to mark them after copy/pasting out of the storybook :)

@surfdude29
Copy link
Contributor

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:

social-app/src/routes.ts

Lines 38 to 42 in 4c3c10d

Support: '/support',
PrivacyPolicy: '/support/privacy',
TermsOfService: '/support/tos',
CommunityGuidelines: '/support/community-guidelines',
CopyrightPolicy: '/support/copyright',

onChange={value => setHapticsDisabled(value)}
style={[a.w_full]}>
<Toggle.LabelText style={[a.flex_1]}>
<Trans>Disable haptic feedback</Trans>
Copy link
Contributor

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants