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

feat: Add quick settings menu with theme switcher and other changes #673

Merged
merged 34 commits into from
Oct 19, 2021
Merged

feat: Add quick settings menu with theme switcher and other changes #673

merged 34 commits into from
Oct 19, 2021

Conversation

amanharwara
Copy link
Member

@amanharwara amanharwara commented Oct 10, 2021

This PR adds a quick settings menu with a theme switcher when there are themes available, otherwise it directly opens the Preferences screen.

Theme Switcher:

image

Other changes:

  • Add arrow-key based navigation to the Account Menu.
  • Close account menu on Escape key if the general menu is open, or else go back to the previous pane.

@amanharwara amanharwara marked this pull request as ready for review October 10, 2021 13:42
@moughxyz moughxyz requested review from gorjan5sk and antsgar October 10, 2021 22:00
@amanharwara amanharwara changed the title feat: Add quick settings menu with theme switcher feat: Add quick settings menu with theme switcher and other changes Oct 12, 2021
@amanharwara amanharwara requested a review from moughxyz October 12, 2021 17:40
@moughxyz moughxyz requested review from antsgar and removed request for antsgar October 13, 2021 15:28
Comment on lines 71 to 75
const items: NodeListOf<HTMLButtonElement> =
accountMenuRef.current.querySelectorAll('.sn-dropdown-item');
const currentFocusedIndex = Array.from(items).findIndex(
(btn) => btn === document.activeElement
);
Copy link
Contributor

Choose a reason for hiding this comment

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

A more React-ish way of doing this would be to have an array of refs, a state variable specifying the currentFocusedIndex and a useEffect hook that would focus the right ref whenever the focused index state variable changes

Copy link
Member Author

Choose a reason for hiding this comment

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

I've implemented this in commit 5219013, but I am facing a weird issue - the "Sign out" button gets focused by default as it gets pushed to the array before "Account settings" does. Ideally, "Account settings" would be focused by default, but I'm not sure what the best way to do that would be with this refs array implementation.

app/assets/javascripts/components/QuickSettingsMenu.tsx Outdated Show resolved Hide resolved
app/assets/javascripts/components/QuickSettingsMenu.tsx Outdated Show resolved Hide resolved
@amanharwara amanharwara merged commit c8dc07d into standardnotes:develop Oct 19, 2021
@amanharwara amanharwara deleted the feat/theme-switcher branch October 19, 2021 04:28
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.

3 participants