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: Introduce item template to PaneHolderPage.xaml #11955

Closed
yaira2 opened this issue Apr 3, 2023 · 9 comments · Fixed by #15544
Closed

Code Quality: Introduce item template to PaneHolderPage.xaml #11955

yaira2 opened this issue Apr 3, 2023 · 9 comments · Fixed by #15544
Assignees
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability)

Comments

@yaira2
Copy link
Member

yaira2 commented Apr 3, 2023

Description

The multipane feature consists of two ModernShellPages and a GridSplitter. The code around this is hard to work with and prevents us from expanding the feature to allow more than two panes. Creating an item template will allow us to create and close panes on demand.

Concerned code

PaneHolderPage.xaml

Gains

  • easier to maintain
  • will allow supporting more than two mains

Requirements

  • use item template instead of hardcoding the right and left pane

Comments

No response

@yaira2 yaira2 added the codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) label Apr 3, 2023
@yaira2 yaira2 changed the title Code Quality: Introduce item template for the multipane feature Code Quality: Introduce item template to PaneHolderPage.xaml Apr 3, 2023
@0x5bfa 0x5bfa self-assigned this Feb 9, 2024
@0x5bfa 0x5bfa removed their assignment Feb 18, 2024
@0x5bfa
Copy link
Member

0x5bfa commented May 6, 2024

What about using BladeView like ColumnsLayout? If we do so, it'd be easier to merge ColumnShellPage and ModernShellPage.

@yaira2
Copy link
Member Author

yaira2 commented May 6, 2024

Once there is a template, we can do some testing to figure out the best control.

As a sidenote, isn't BladeView being phased out?

@0x5bfa
Copy link
Member

0x5bfa commented May 6, 2024

Yeah, they plan to remove.
We can make similar one if we plan not to use as well.

Agreed, let’s create a template first.

@0x5bfa
Copy link
Member

0x5bfa commented May 8, 2024

I did a quick search and I found that we cannot add time template and item repeater to enumerate shell pages as we have sizer in-between.

We seem to have use StackPanel.Children and add UIElements to it. This increases amount of code in code behind but this seems to be a better idea.

Things what we have to interact with are:

  • Add a pane
  • Remove a pane
  • Focus on a pane using accent border color at the bottom.
  • Focus off a pane
  • Support to resize panes

Current GridSpitter is hard to handle since we have to manipulate every interaction, while WCT 8.0's ContentSizer has brilliant feature, which it targets a sizing control internally:

<StackPanel x:Name="LeftPanel" />
<wct:ContentSizer Target="{Binding LeftPanel}" />

This may reduce a certain amount of code and improve maintainability. Also removing the concept of 'left' or 'right' enables user to remove any pane at any time.

@0x5bfa 0x5bfa self-assigned this May 8, 2024
@yaira2
Copy link
Member Author

yaira2 commented May 8, 2024

We might want to support vertical panes as well in the future.

@0x5bfa
Copy link
Member

0x5bfa commented May 8, 2024

Let's use StackPanel.Orientation then

@yaira2
Copy link
Member Author

yaira2 commented May 8, 2024

There can be a scenario with 4 panes, two on top, and two on bottom.

@0x5bfa
Copy link
Member

0x5bfa commented May 8, 2024

Oh are we gonna support different orientations?

+-----------+-----------+
|           |           |
|  Top (L)  |  Top (R)  |
|           |           |
+-----------+-----------+
|           |           |
| Bottom(L) | Bottom(R) |
|           |           |
+-----------+-----------+

@yaira2
Copy link
Member Author

yaira2 commented May 9, 2024

After discussing Discord, we'll only support one orientation at a time. It's also worth pointing out that we're not actually adding support for different orientations right now, we're only bringing this up in order to prevent extra work in the future.

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: Done
Development

Successfully merging a pull request may close this issue.

2 participants