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

Code Quality: Broader SidebarView control usage #15283

Open
0x5bfa opened this issue May 1, 2024 · 2 comments
Open

Code Quality: Broader SidebarView control usage #15283

0x5bfa opened this issue May 1, 2024 · 2 comments
Assignees
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability)

Comments

@0x5bfa
Copy link
Member

0x5bfa commented May 1, 2024

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

  • Around SidebarView class

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:
main...0x5bfa:Files:5bfa/CQ-RefactorSidebarViewModel

@0x5bfa 0x5bfa added the codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) label May 1, 2024
@0x5bfa 0x5bfa self-assigned this May 1, 2024
@0x5bfa
Copy link
Member Author

0x5bfa commented May 4, 2024

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

@0x5bfa
Copy link
Member Author

0x5bfa commented May 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability)
Projects
Status: 📋 Planning stage
Development

No branches or pull requests

1 participant