Skip to content

Add a separate Settings button inside the top bar #1477

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.

Sign up for GitHub

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

Jump to bottom
Merged
sejas merged 8 commits into from
Jun 13, 2025
Merged

Add a separate Settings button inside the top bar #1477

sejas merged 8 commits into from
Jun 13, 2025

Conversation

@sejas
Copy link
Member

@sejas sejas commented Jun 11, 2025 •
edited
Loading

Related issues

Proposed Changes

  • Add a new Settings button to open the modal.
  • The login button opens the OAuth webpage directly.
  • When the user is authenticated, both buttons (WPcom Avatar and Settings) open the settings modal but with different tabs selected: Account and Preferences, respectively.
  • "Cmd + ," , and Menu > Studio > Settings selects the preferences tab by default
  • Studio assistant usage limit message selects the usage tab by default.

Testing Instructions

  • Run npm start
  • Observe a new Settings button on the top right corner, before the help icon.
  • Click on the new Settings button
  • Observe the settings modal opens and the Preferences tab is selected.
  • Go to the Account tab and click log in from the modal
  • Approve
  • Observe the deep link opens Studio and the modal is still open
  • Click on Log out
  • Close the modal
  • Click on Login in the main screen
  • Observe the OAuth opens directly (without any modal)
  • Approve
  • Observe the deep link opens Studio and no modal is present
  • Click on your avatar image
  • Observe the settings modal also opens and the account tab is preselected.
pre-select-settings-tab.mp4

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@sejas sejas self-assigned this Jun 11, 2025
@sejas sejas requested a review from a team June 11, 2025 18:55
nightnei
nightnei requested changes Jun 11, 2025
View reviewed changes
Copy link
Contributor

@nightnei nightnei left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM and works well, but I have concern regarding UX - we have two buttons inside the same menu that behave totally the same way. It feels extra. Asked about thoughts / ideas here p1749673809988249/1749644397.678369-slack-C06DRMD6VPZ

So temporarily mark it as "Requested changes"

ivan-ottinger
ivan-ottinger approved these changes Jun 12, 2025
View reviewed changes
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment

Choose a reason for hiding this comment

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

The changes look good and work as expected. This is great improvement! 👌🏼

I have followed the related Slack discussion and shared one additional idea there (p1749716191737259/1749644397.678369-slack-C06DRMD6VPZ).

Similarly to other, I also think we could consider swapping the ❔ and ⚙️ icons.

katinthehatsite
katinthehatsite reviewed Jun 12, 2025
View reviewed changes
Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

In terms of functionality, everything works as described in the PR 👍

In terms of the UI experience, I find it a bit strange (similarly to what Vova mentioned) as we now have two buttons that do the exact same thing when the user is logged in.

I am wondering if we might want to split the functionality of the Log in and Preference so that when the user is logged in, clicking Gravatar would look them out. And the preferences would open through the settings button instead. Or something along these lines so that the functionality of these buttons is not repeated.

What do you think?

@wojtekn
Copy link
Contributor

wojtekn commented Jun 12, 2025

@sejas looks good and it works well. What if we try to implement the solution we discussed on Slack to resolve the confusion caused by two elements opening the same UI? We could make the Settings icon open the Preferences tab in the Settings dialog, and leave the 'Howdy...' opening the Account tab.

I think we can switch the Help icon with the Settings icon, too, as having the Help at the end of the bar may be clearer.

@sejas
Copy link
Member Author

sejas commented Jun 12, 2025

@nightnei , @ivan-ottinger , @katinthehatsite , thanks for the review. We followed Ivan's suggestion and we'll open thespecific tab for authenticated users.
@wojtekn , makes sense. I updated the code, screencast and description.
Feel free to have a second review.

@sejas sejas requested review from a team and nightnei June 12, 2025 16:01
wojtekn
wojtekn approved these changes Jun 13, 2025
View reviewed changes
Copy link
Contributor

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

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

Works well, and all flows related to authentication and settings look clear.

One thing we could improve is to open Preferences tab when user opens Setting using CMD+, shortcut.

ivan-ottinger
ivan-ottinger approved these changes Jun 13, 2025
View reviewed changes
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the updates, Antonio! The proposed changes look good and the buttons work correctly.

Log in button goes directly to WordPress.com Log in page (or the Account settings tab) and ⚙️ points to the Preferences tab directly. 👌🏼🙂

I did not observe any regressions.

While testing VoiceOver, I noticed that the buttons are announced as follows now:

  • Log in button: Open settings, button, group or Open settings to log in, button, group
  • ⚙️ button: Settings, button
  • ❔ button: Get help, button

I think we could consider updating the Log in button to something like: Log in to Studio with WordPress.com, button and Open account settings, button. But we could address this separately of course, outside of this PR.

src/modules/user-settings/components/user-settings.tsx
Comment on lines +79 to +85
if ( isAuthenticated ) {
tabs.push( {
name: 'usage',
title: __( 'Usage' ),
} );
}

Copy link
Contributor

@ivan-ottinger ivan-ottinger Jun 13, 2025

Choose a reason for hiding this comment

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

Nice refactor!

@sejas
Copy link
Member Author

sejas commented Jun 13, 2025

Works well, and all flows related to authentication and settings look clear.

One thing we could improve is to open Preferences tab when user opens Setting using CMD+, shortcut.

Great idea! Done! 4872fff

Thank you for the updates, Antonio! The proposed changes look good and the buttons work correctly.

Log in button goes directly to WordPress.com Log in page (or the Account settings tab) and ⚙️ points to the Preferences tab directly. 👌🏼🙂

I did not observe any regressions.

While testing VoiceOver, I noticed that the buttons are announced as follows now:

  • Log in button: Open settings, button, group or Open settings to log in, button, group
  • ⚙️ button: Settings, button
  • ❔ button: Get help, button

I think we could consider updating the Log in button to something like: Log in to Studio with WordPress.com, button and Open account settings, button. But we could address this separately of course, outside of this PR.

Good catch! I like that we now pay special attention to A11y. Done! 6f03a66

@sejas sejas merged commit a45867d into trunk Jun 13, 2025
13 checks passed
@sejas sejas deleted the add/stu-566-untangle-studio-settings-from-log-in branch June 13, 2025 12:16
@ivan-ottinger
Copy link
Contributor

ivan-ottinger commented Jun 13, 2025

Good catch! I like that we now pay special attention to A11y. Done! 6f03a66

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@katinthehatsite katinthehatsite katinthehatsite left review comments

@wojtekn wojtekn wojtekn approved these changes

@ivan-ottinger ivan-ottinger ivan-ottinger approved these changes

@nightnei nightnei Awaiting requested review from nightnei

Assignees

@sejas sejas

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sejas @wojtekn @ivan-ottinger @nightnei @katinthehatsite