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.

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

Feature: Redesigned the left hand sidebar #13052

Merged
merged 106 commits into from
Aug 13, 2023
Merged

Feature: Redesigned the left hand sidebar #13052

merged 106 commits into from
Aug 13, 2023

Conversation

marcelwgn
Copy link
Contributor

@marcelwgn marcelwgn commented Jul 24, 2023 •
edited
Loading

This PR migrates the SideBar implemenation away from the NavigationView control to our own written control, making feature development and styling a lot easier.

Fixes #12930

ToDos:

  • Update minimal pane to cover entire side (if desired)?
  • Animate expanding/collapsing pane
  • Animate expanding/collapsing items
  • Update styling of sidebar to match design
  • Keyboard handling
  • Proper drag and drop handling
  • Fix initial state being wrong
  • Fix cursor when hovering over the Grid Splitter
  • Add back double click (on the Grid Splitter) to collapse/expand the sidebar
  • Fix issue where right clicking item triggers a click
  • Add tooltips (big tooltips for drives are another issue)
  • Reordering favourites might crash and reset favourites
  • Double clicking resizer leaves it in weird visual state
  • Restore pane width between sessions
  • Restore compact mode between sessions
  • Fix icon in the network section
  • Fix the tooltip for OneDrive

Screenshots (optional)
Animation showing the different behaviors for the new sidebar

@marcelwgn marcelwgn requested a review from yaira2 July 24, 2023 09:46
@marcelwgn
Copy link
Contributor Author

@mdtauk and @0x5bfa, I would love your feedback on this!

@mdtauk
Copy link
Contributor

mdtauk commented Jul 24, 2023

Is there a reason you have gone with a UserControl instead of a Custom Control?

@marcelwgn
Copy link
Contributor Author

Is there a reason you have gone with a UserControl instead of a Custom Control?

The performance benefit for a single use control is neglible in my opinion compared to the ease of development you get with a UserControl.

@mdtauk
Copy link
Contributor

mdtauk commented Jul 24, 2023

Will this control not eventually be used for the Settings UI and for the Properties dialog?

@marcelwgn
Copy link
Contributor Author

Will this control not eventually be used for the Settings UI and for the Properties dialog?

This was not the initial plan but I don't see a reason why not. Of course, @yaira2 has the say on this one though.

@0x5bfa

This comment was marked as outdated.

Sign in to view
@marcelwgn

This comment was marked as resolved.

Sign in to view
@yaira2

This comment was marked as resolved.

Sign in to view
@0x5bfa

This comment was marked as outdated.

Sign in to view
@marcelwgn

This comment was marked as resolved.

Sign in to view
@yaira2

This comment was marked as resolved.

Sign in to view
@0x5bfa
Copy link
Member

0x5bfa commented Jul 24, 2023

NICEEE!!!

@yaira2
Copy link
Member

yaira2 commented Jul 24, 2023

Update minimal pane to cover entire side

I'm okay with the implementation as is and I think it'll be easier to support the control without having to cover the entire side.

@mdtauk

This comment was marked as resolved.

Sign in to view
@mdtauk

This comment was marked as resolved.

Sign in to view
@yaira2
Copy link
Member

yaira2 commented Jul 24, 2023

I think placing it into the address bar, before the back and forward buttons makes the most sense, with the sidebar sliding in below.

This is fine with me 👍

@yaira2

This comment was marked as resolved.

Sign in to view
@mdtauk

This comment was marked as resolved.

Sign in to view
@yaira2
Copy link
Member

yaira2 commented Jul 24, 2023

So the visual and how that would work, is not related to the Resizing of the sidebar?

It'll be the same as the current design

@Lukiluc29
Copy link
Contributor

I wanted to inquire if it's normal that the tooltip displaying the size of the Recycle Bin has disappeared.

@marcelwgn
Copy link
Contributor Author

This is indeed not intentional (it only is for drives/cloud drives), I've fixed this now. Thank you for pointing that out @Lukiluc29 !

@Lukiluc29
Copy link
Contributor

The sidebar looks pretty good to me!

@Lukiluc29
Copy link
Contributor

Since you have disabled cloud disk tooltips because they are erroneous, this issue #11326 can be closed

@marcelwgn
Copy link
Contributor Author

@yaira2 Would we like to reimplement the tooltips for disks/cloud drives to the way they were before or can #11326 be truly closed then?

@yaira2
Copy link
Member

yaira2 commented Aug 11, 2023

@chingucoding I'm fine without reimplementing the rich tooltips, the plan is to display this info in the details pane instead.

@mdtauk
Copy link
Contributor

mdtauk commented Aug 11, 2023

@chingucoding I'm fine without reimplementing the rich tooltips, the plan is to display this info in the details pane instead.

What was the issue with displaying the tooltip? Performance? Reliability of the data?

@marcelwgn
Copy link
Contributor Author

What was the issue with displaying the tooltip? Performance? Reliability of the data?

Mostly the issue how to separate the SidebarView from the Items and then somehow generating those tooltips based on the items.

@Lukiluc29
Copy link
Contributor

I'm conducting some tests to ensure there are no new sidebar-related bugs, and I've noticed that when we rearrange favorites and the "Favorites" section collapses, it displays as it used to, with a favorites icon instead of just using text. However, it reverts back to the correct current and rearranged display afterward.
Capture d'écran 2023-08-12 083519

@marcelwgn
Copy link
Contributor Author

I'm conducting some tests to ensure there are no new sidebar-related bugs, and I've noticed that when we rearrange favorites and the "Favorites" section collapses, it displays as it used to, with a favorites icon instead of just using text. However, it reverts back to the correct current and rearranged display afterward.

I'm inclined to say this is somewhat intentional and correct (at least from a SidebarView perspective). When we reorder the favorites, we essentially remove all favorites from the sidebar and than readd them. While there are no children, the favorite item acts as a top level element. @yaira2 @mdtauk What are your thoughts on this behavior?

@mdtauk
Copy link
Contributor

mdtauk commented Aug 12, 2023 •
edited
Loading

Ideally the item would know it's a group header, and will contain items when they are added to the group.

@marcelwgn
Copy link
Contributor Author

But what if there are no child items, do we show the header and nothing else? Doesn't that invite users to click on it despite the section itself not making much sense?

@mdtauk
Copy link
Contributor

mdtauk commented Aug 12, 2023

But what if there are no child items, do we show the header and nothing else? Doesn't that invite users to click on it despite the section itself not making much sense?

  • We could hide the chevron when there are no children
  • Hide the group entirely
  • Add a label with text like "Pin folders here for quick access" or "No network locations are available" etc

@marcelwgn
Copy link
Contributor Author

We could hide the chevron when there are no children

I went with this solution for now.

@mdtauk
Copy link
Contributor

mdtauk commented Aug 12, 2023

We could hide the chevron when there are no children

I went with this solution for now.

What appears in the flyout, for an empty group when the sidebar is in the Collapsed mode?

@marcelwgn
Copy link
Contributor Author

What appears in the flyout, for an empty group when the sidebar is in the Collapsed mode?

I've updated the behavior to don't do anything, before that it would have shown an empty flyout. What do you think is better for users?

@marcelwgn
Copy link
Contributor Author

And also, in my opinion the correct behavior for reordering would be to not actually remove all items and readd them but just insert the items in the correct position so we wouldn't walk into that situation at all.

@mdtauk
Copy link
Contributor

mdtauk commented Aug 13, 2023

What appears in the flyout, for an empty group when the sidebar is in the Collapsed mode?

I've updated the behavior to don't do anything, before that it would have shown an empty flyout. What do you think is better for users?

If there is a label for the empty group state, this label can be displayed in the flyout.

yaira2
yaira2 approved these changes Aug 13, 2023
View reviewed changes
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2
Copy link
Member

yaira2 commented Aug 13, 2023 •
edited
Loading

Since you have disabled cloud disk tooltips because they are erroneous, this issue #11326 can be closed

@Lukiluc29 removing the tooltip doesn't fix the underlying issue so we'll want to keep that issue open.

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Aug 13, 2023
@yaira2 yaira2 merged commit 9c34c31 into files-community:main Aug 13, 2023
@marcelwgn marcelwgn deleted the dev/new-sidebar branch August 13, 2023 17:30
yaira2 added a commit to QuaintMako/Files that referenced this pull request Aug 16, 2023
@yaira2
Squashed commit of the following:
15e2621 
commit 2b4132f
Author: Filippo Ferrario <[email protected]>
Date:   Tue Aug 15 23:02:52 2023 +0200

    Code Quality: Create alike RichCommands inheritance (files-community#13155)

commit bd8c78f
Author: hishitetsu <[email protected]>
Date:   Wed Aug 16 05:50:24 2023 +0900

    Fix: Fixed issue where deleted items were displayed in tag search results (files-community#13202)

commit 4b5ed37
Author: hishitetsu <[email protected]>
Date:   Tue Aug 15 03:27:18 2023 +0900

    Fix: Fixed issue where closing the last closed tab would open it when starting the next session (files-community#13198)

commit 6e37d69
Author: Marco Franzen <[email protected]>
Date:   Mon Aug 14 16:17:14 2023 +0200

    Fix: Fixed crash that would occur when deleting or restoring files (files-community#13195)

commit 001e8eb
Author: Yair <[email protected]>
Date:   Sun Aug 13 12:29:35 2023 -0400

    Preview: v2.5.22 (files-community#13190)

commit f77259a
Author: d2dyno <[email protected]>
Date:   Sun Aug 13 17:28:48 2023 +0200

    Fix: Fixed issue where items in the Tags widget were not localized (files-community#13149)

commit 9c34c31
Author: Marcel Wagner <[email protected]>
Date:   Sun Aug 13 16:20:48 2023 +0200

    Feature: Redesigned the left-hand sidebar (files-community#13052)

commit 3d88463
Author: Lukiluc29 <[email protected]>
Date:   Sun Aug 13 16:20:24 2023 +0200

    Fix: Fixed the corner radius on the drive details page (files-community#13179)

commit 89e3d03
Author: 0x5BFA <[email protected]>
Date:   Thu Aug 10 09:14:38 2023 +0900

    Feature: Updated the design of the home page headers (files-community#13053)

commit f02c4a0
Author: Yair <[email protected]>
Date:   Wed Aug 9 17:16:26 2023 -0400

    Feature: Increased the default width of the Git status column (files-community#13168)

commit c979c22
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Aug 9 00:55:38 2023 -0400

    Build(deps): Bump Microsoft.Data.Sqlite.Core from 7.0.9 to 7.0.10 (files-community#13163)

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit beae745
Author: hishitetsu <[email protected]>
Date:   Wed Aug 9 12:01:49 2023 +0900

    Fix: Fixed issue where the back button in the Properties window wasn't working properly (files-community#13162)

commit 7f52809
Author: Filippo Ferrario <[email protected]>
Date:   Tue Aug 8 19:03:10 2023 +0200

    Code Quality: Fixed issue with clicking search box (files-community#13158)

    Co-authored-by: hishitetsu <[email protected]>

commit 311b380
Author: Filippo Ferrario <[email protected]>
Date:   Tue Aug 8 17:27:37 2023 +0200

    Fix: Fixed an issue where clicking an empty space would scroll to the top of the file list (files-community#13157)

commit 0988113
Author: Filippo Ferrario <[email protected]>
Date:   Tue Aug 8 17:21:34 2023 +0200

    Feature: Show BitLocker options in main menu for drives (files-community#13142)

commit 3221267
Author: Filippo Ferrario <[email protected]>
Date:   Tue Aug 8 17:17:48 2023 +0200

    Feature: Display error message when transferring files that are too large for FAT32 (files-community#13137)

commit be42aea
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Aug 8 10:30:20 2023 -0400

    Build(deps): Bump Vanara.Windows.Shell from 3.4.15 to 3.4.16 (files-community#13153)

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 53d99af
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Aug 8 08:37:00 2023 -0400

    Build(deps): Bump Vanara.PInvoke.Mpr from 3.4.15 to 3.4.16 (files-community#13152)

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 43624b3
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Aug 8 08:36:50 2023 -0400

    Build(deps): Bump Vanara.Windows.Extensions from 3.4.15 to 3.4.16 (files-community#13151)

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit e7281fe
Author: Yair <[email protected]>
Date:   Mon Aug 7 21:39:09 2023 -0400

    Fix: Fixed issue where clicking DataGrid headers didn't change the sort direction (files-community#13150)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0x5bfa 0x5bfa 0x5bfa left review comments

@yaira2 yaira2 yaira2 approved these changes

Assignees
No one assigned
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: Display sidebar options when right clicking headers
6 participants
@marcelwgn @mdtauk @0x5bfa @yaira2 @Lukiluc29 @d2dyno1