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

chore(web): context menu improvements #10475

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Conversation

ben-basten
Copy link
Member

@ben-basten ben-basten commented Jun 19, 2024

Description

Enhancements for context menu options:

  • configurable active color - when hovered, or navigated to with arrow keys
  • configurable menu option text color
  • remove slots - the "subtitle" slot was unused, and the default slot was only used in 1 place
  • onClick callbacks rather than firing events
  • improved layout - icon is vertically centered, and subtitle text no longer runs underneath the icon

Also in this change:

  • migrate shared album activity sidebar to the ButtonContextMenu component

How Has This Been Tested?

Opening dropdowns in both light mode and dark mode using combinations of options that have:

  • no icon with title
  • no icon with title and subtitle
  • icon with title
  • icon with title and subtitle
  • icons with title and a custom text color
  • icons with title, subtitle and a custom text color (left the subtitle hardcoded to gray in this scenario)

For shared albums, I also tried removing comments and likes logged in as both the album owner and the shared user.

Screenshots

Below are a couple of different flavors of what a MenuOption can look like.

External library dropdown

Dropdown, with delete option hovered to show the custom active color

external-library-menu-v2

Mock scenario to demo other menu option layouts

Mock scenario:

  • icon, title, and subtitle in the same option
  • icon and title in a custom text color

external-library-mock

Activity viewer

activity-menu

- ability to add custom hover colors
- migrate activity menu to ButtonContextMenu component
- onClick callbacks rather than events for menu options
- remove slots
- configurable menu option colors
- improve menu option layout
@alextran1502 alextran1502 merged commit 0fda675 into main Jun 20, 2024
23 checks passed
@alextran1502 alextran1502 deleted the chore/context-menu-followup branch June 20, 2024 21:15
@waclaw66
Copy link
Contributor

Selected menu item is too dark now, the selection color was much better before, in my opinion...

before PR
obrazek obrazek

@ben-basten
Copy link
Member Author

Hey @waclaw66, thanks for the feedback!

In my opinion, the dropdown selection color should stay as-is to make the option easier to see due to the higher color contrast.

For reference, here is the comparison of the colors that Google Photos uses vs. immich, using the WebAIM contrast checker to calculate ratios:

Google Photos

Inactive Active Contrast ratio
Current rgb(240,244,249)
#F0F4F9
rgb(215,218,222)
#D7DADE
1.26:1

immich

Inactive Active Contrast ratio
Before bg-slate-100
rgb(241,245,249)
#F1F5F9
bg-gray-200
rgb(229,231,235)
#E5E7EB
1.13:1
Current bg-slate-100
rgb(241,245,249)
#F1F5F9
bg-slate-300
rgb(203,213,225)
#CBD5E1
1.35:1

The immich contrast ratio of 1.13:1 for the prior implementation of the dropdown is not very high, so I suggest using darker colors to ensure visibility for as many users as possible. Google's contrast ratio of 1.26:1 feels like a good minimum target.

@waclaw66
Copy link
Contributor

Hi @ben-basten, thanks for detailed explanation. I agree that the contrast is bit low, however I was surprised with the bluish accent of the selection. But it's all about habit.

@ben-basten
Copy link
Member Author

I agree that the contrast is bit low, however I was surprised with the bluish accent of the selection.

Yeah, it is a bluer color. An option is to check and see what bg-gray-300 looks like.

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

Successfully merging this pull request may close these issues.

4 participants