-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Redesign library navigation in experimental layout #6582
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
Redesign library navigation in experimental layout #6582
Conversation
Cloudflare Pages deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
src/apps/experimental/components/AppToolbar/userViews/UserViewNav.tsx
Outdated
Show resolved
Hide resolved
src/apps/experimental/components/AppToolbar/userViews/UserViewNav.tsx
Outdated
Show resolved
Hide resolved
src/apps/experimental/components/AppToolbar/userViews/UserViewNav.tsx
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
11f0cb7
to
ab5a72d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
ab5a72d
to
568ee6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
568ee6f
to
56c6b19
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
56c6b19
to
f23fcd0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
f23fcd0
to
96b3173
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
I know I'm being a bit pedantic here but the upper and bottom padding doesn't appear the same on firefox. Edit: I think it's a weird firefox thing, it looks fine on safari But here is a better view, it's 5px top vs 4px bottom. Here it's nicely 4px by 4px shakes fist at firefox, this reminds me of the good old IE vs Phoenix vs Opera days. |
96b3173
to
e095b02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
e095b02
to
30a129d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
30a129d
to
7df3142
Compare
05c72ad
to
ac8593d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint doesn't pass. Please fix all ESLint issues.
ac8593d
to
b43d0a2
Compare
b43d0a2
to
d5cdf77
Compare
d5cdf77
to
c5da93c
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/apps/experimental/AppOverrides.scss: Language not supported
Comments suppressed due to low confidence (1)
src/apps/experimental/features/libraries/constants/libraryRoutes.ts:168
- Multiple views in the '/homevideos' route are marked as 'isDefault'. This could lead to ambiguity when determining the default view; consider ensuring only one default per route.
isDefault: true
I personally preferred the drop-down approach of yester-pull for switching between different libraries, especially considering the amount of libraries different users have may vary greatly and is done less frequently during navigation. I do however agree that the other section navigation options should be listed in full as done here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the biggest fan of the design but I don't have any better ideas right now. Approving for the sake of progression, further changes can always be made in new pull requests. It's called experimental for a reason!
Been using it every now and then, it's a small step up from the current experimental IMHO. In do miss the 1 click to get to dashboard from the old experimental though. |
It definitely won't be the final design (our design lead is working on a full redesign). It's just meant to address the layout issues caused by the pinned sidebar in the meantime. 😉 |
Note
These changes only applies to the experimental layout
Changes
Library navigation has been moved to the header with the order following the user settings:

If there are too many views, then the remaining options go into a "More" drop down menu:

When viewing a library, the text for that button is highlighted. The library view options were moved to a drop down menu replacing tabs and the alignment of the pagination and view options were updated:

The primary objective for this was to reduce the amount of space wasted by the pinned sidebar in the experimental layout that causes a lot of layout issues on smaller screens.
The mobile header is unchanged.
Issues
N/A