search

files-community / Files

Building the best file manager for Windows
https://files.community
MIT License
33.01k stars 2.11k forks source link

Code Quality: Broader SidebarView control usage #15283

Open 0x5bfa opened 2 months ago

0x5bfa commented 2 months ago

Description

We're planning to use Sidebar control for consistency and simplicity in properties window and settings window. I already confirmed this can reduce 200 lines.

Concerned code

Gains

Consistency and simplicity

Requirements

This work appears to be huge, so I suggest to break down to a few phases below:

  1. Rename sidebar item classes such as DriveItem to be StandardDriveItem. In the future those items will also be used in widgets and even layout pages.
  2. Separation
    • Separate view-unique values such as ExpandedWidthThreshold, IsPaneOpen and ViewModel. Note that we probably need to separate almost all dependency properties in SidebarView.
    • Separate view-unique events such as ItemInvokedAsync.
  3. Add generation method for each Sidebar type: Default, Properties, Settings.
  4. Create FrameworkElement property to hold icon element for both BitmapImage and OpacityIcon

Comments

My prototype: https://github.com/files-community/Files/compare/main...0x5bfa:Files:5bfa/CQ-RefactorSidebarViewModel

0x5bfa commented 2 months ago

@yaira2 Can I work on refactoring SidebarViewModel? It has been unchanged from UWP.

0x5bfa commented 1 month ago

https://discord.com/channels/725513575971684472/958783217941487616/1236699756265214052

At the end of quick discussion, we settled on the idea to generalize usage of SidebarView, moving view-specific properties, such as item collection, pane mode, pane width. This also enables us to bind values to user settings to make them keep up with the latest info all the time. The new usage will be almost the same as the one of NavigationView, such as invoking an event to notify item invocation, using two-way binding for selected item.

In this issue, I guess we also might as well discuss the idea to make items larger to the height probably based on a user setting - EnableCompactModeInSidebar.