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

Merged
merged 35 commits into from
Oct 29, 2024
Merged

Settings revamp #5745

merged 35 commits into from
Oct 29, 2024

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

@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 14, 2024 12:37 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 14, 2024 12:43 — with Render Destroyed
Copy link

github-actions bot commented Oct 14, 2024

Old size New size Diff
7.92 MB 7.98 MB 60.1 KB (0.74%)

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 :)

@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 15, 2024 10:46 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 15, 2024 10:48 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 15, 2024 11:00 — with Render Destroyed
@mozzius mozzius marked this pull request as ready for review October 15, 2024 11:58
@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',

@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 28, 2024 15:08 — with Render Destroyed
@haileyok
Copy link
Contributor

Can we stick some padding at the bottom of this screen? Final option feels a bit too close to the bottom.

Screenshot 2024-10-29 at 12 47 18 PM

@haileyok
Copy link
Contributor

Nit. I think it feels a little off that the toggle isn't aligned with the first line here.

Screenshot 2024-10-29 at 12 48 17 PM

@haileyok
Copy link
Contributor

Padding is off here on web

Screenshot 2024-10-29 at 12 51 24 PM

@haileyok
Copy link
Contributor

haileyok commented Oct 29, 2024

Wonder if we could remove the inner border from this on web? Also let's add a close button maybe.

Screenshot 2024-10-29 at 12 52 03 PM

@haileyok
Copy link
Contributor

Let's drop the background color on this (maybe you did in one of the following PRs though in which case ignore me)

Screenshot 2024-10-29 at 12 53 17 PM

Copy link
Contributor

@haileyok haileyok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exceptionally good work. Sorry it took so long to start picking it up for review.

Looks good to go after checking the nits above.

]}>
<Trans>Logged-out visibility</Trans>
</Text>
{!IS_INTERNAL && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm figured it out

@@ -210,6 +210,7 @@ export function Component({}: {}) {
to="#"
onPress={e => {
e.preventDefault()
closeModal()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didnt look closely, but is this meant to be here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's broken on main lol

image

@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 29, 2024 21:01 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 29, 2024 21:02 — with Render Destroyed
@arcalinea arcalinea temporarily deployed to samuel/settings - social-app PR #5745 October 29, 2024 21:10 — with Render Destroyed
@mozzius mozzius merged commit c8f264b into main Oct 29, 2024
6 checks passed
@mozzius mozzius deleted the samuel/settings branch October 29, 2024 21:14
estrattonbailey added a commit that referenced this pull request Oct 31, 2024
* origin/main: (213 commits)
  add Thai Language translation support (#5879)
  fix warning on labeler profile: emoji detected but emoji not enabled (#6011)
  Added blur to search's onPressMenu (#6017)
  React compiler beta and reenable rule (#5898)
  Sort imports (#6009)
  Clarify build instructions (#6008)
  Upgrade all tiptap deps to latest (#5232)
  width full on text container (#6007)
  Bump 1.94.0 (#6006)
  Add subtle web hover to interactive rows (#5989)
  Settings revamp (#5745)
  Show almost-instant preview when opening lightbox (#6000)
  Refactor lightbox model to plain object (#5999)
  temp revert to old modal (#6005)
  Extend composer checks to all posts in a thread (#5955)
  Remove indirection when rendering composer state (#5954)
  Refactor composer state for threads (#5945)
  Disable Post button when empty (#5953)
  fix splash screen (#5997)
  Fix video quality for short videos (#5996)
  ...
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
* start building storybook

* add title

* add some styles

* try out new icons

* more settings list component parts

* make text do the spacing

* clean up storybook

* gated new settings screen

* switch account

* add current profile

* use Layout.Screen

* Layout.Header and Layout.Content

* translate helpdesk text

thanks @surfdude29!

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* add account settings

* undo changes to export car dialog

* privacy and security screen

* Translate protect account stuff

Thanks @surfdude29!

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* content and media settings

* about settings

* 2fa copy

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* a11y and appearance

* use new components for appearance settings

* redesign accessibility settings

* Update ContentAndMediaSettings.tsx

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* add divider

* remove a11y and appearance middleman screen

* fix web settingslist styles

* new SettingsList.Group component

* explain how portal magic works

* hide pwioptout in old location

* Update Settings.tsx

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* replace gate with `IS_INTERNAL`

* add IS_INTERNAL to app-info.web

* fix profile area growing

* add close button to switch account

---------

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
* start building storybook

* add title

* add some styles

* try out new icons

* more settings list component parts

* make text do the spacing

* clean up storybook

* gated new settings screen

* switch account

* add current profile

* use Layout.Screen

* Layout.Header and Layout.Content

* translate helpdesk text

thanks @surfdude29!

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* add account settings

* undo changes to export car dialog

* privacy and security screen

* Translate protect account stuff

Thanks @surfdude29!

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* content and media settings

* about settings

* 2fa copy

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* a11y and appearance

* use new components for appearance settings

* redesign accessibility settings

* Update ContentAndMediaSettings.tsx

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* add divider

* remove a11y and appearance middleman screen

* fix web settingslist styles

* new SettingsList.Group component

* explain how portal magic works

* hide pwioptout in old location

* Update Settings.tsx

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>

* replace gate with `IS_INTERNAL`

* add IS_INTERNAL to app-info.web

* fix profile area growing

* add close button to switch account

---------

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
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.

5 participants