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

[UI/UX] Improve sidebar #3671

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

Kajot-dev
Copy link
Contributor

@Kajot-dev Kajot-dev commented Apr 2, 2024

This PR is an attempt to de-clutter sidebar and improve it's readibility.

Wating for #3544 and then will rebase this on top of that
Toy can review and QA this UI-wise, but code complexity should be reduced thanks to the mentioned PR

Most notable changes:

  • Sub items now use Accordion
  • Download widget is now part of the "downloads" section
  • In collapsed mode there is a badge instead
  • "Update is Available" is now styled similarly to other elements
  • All menu entries now have an icon
  • In collapsed mode, the sidebar uses tooltips (not the MUI ones tho, they bring maaany issues) that appear on hover or on :focus-visible, so navigation with gamepad should be working
  • Quit button has a danger color
  • There are now slight margins and rounded corners (to match with the other elements of heroic)
  • SidebarLinks got split into MainLink and ExtraLinks
  • Removed runner and appname state from SidebarLinks (MainLink) since game settings are using modal for quite some time (It was dead code)
  • Removed --divider css variable as it was available only in one theme and used only by dividers on the sidebar (other dividers through the heroic do no use it), now relying on --neutral-*
sidebar.webm

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@Kajot-dev Kajot-dev changed the title Improve sidebar [UI/UX] Improve sidebar Apr 2, 2024
@Kajot-dev
Copy link
Contributor Author

Kajot-dev commented Apr 2, 2024

Codecheck failed within install deps, because of an existing npm cache
(this was CSS only commit btw)

@arielj
Copy link
Collaborator

arielj commented Apr 2, 2024

I wouldn't remove --divider, we could rename it to --sidebar-divider-color for example, and, when used, do something like var(--sidebar-divider-color, var(--neutral...), so it has a fallback but can be customized by themes if needed

one thing I tried to do in #3544 is to actually add new CSS variables (or improve names) to allow easier customization by themes but adding fallbacks for better defaults when themes don't define something

I'm not 100% sure about items in an accordion, I know layout shifts can be considered bad for accessibility, I don't have a strong opinion but I also understand there can be concerns about that change

@Kajot-dev
Copy link
Contributor Author

As for the divider, sorry! 😬 I will rebase it and use it as you suggested.

As for the Accorion. It's not like the layout did not shift before. I agree that layout shifting is bad, but (in my opinion) it's better when it at least has an animation. But valid point, maybe someone will come with a better solution (context menu?).

@Kajot-dev
Copy link
Contributor Author

@arielj I missed your PR. If thats okay with you I will mark this PR as a draft and wait until #3544 gets merged and rebase this on top of your changes

@Kajot-dev Kajot-dev marked this pull request as draft April 3, 2024 06:13
@Kajot-dev
Copy link
Contributor Author

This is how "update available" looks:
image

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.

None yet

2 participants