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

MudStack: Add parameter StretchItems to have certain children fill space #8545

Merged
merged 5 commits into from
Apr 2, 2024

Conversation

BieleckiLtd
Copy link
Contributor

@BieleckiLtd BieleckiLtd commented Apr 1, 2024

If the original PR #6010 was rejected due to conflicts, this update brings the previous code up to date with the current dev branch. If the rejection was for a different reason, please close this PR. Thank you.

Description

Closes #4960

2022-12-19_00-36-23.mp4

How Has This Been Tested?

unit + visually

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added enhancement New feature or request PR: needs review labels Apr 1, 2024
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.48%. Comparing base (9cd0827) to head (8d50859).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8545      +/-   ##
==========================================
- Coverage   89.51%   89.48%   -0.03%     
==========================================
  Files         410      410              
  Lines       11831    11832       +1     
  Branches     2355     2355              
==========================================
- Hits        10590    10588       -2     
- Misses        713      715       +2     
- Partials      528      529       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Great work - LGTM

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

I like the functionality, it is very useful, reminds me of LastChildFill in WPF's DockPanel. Before we merge it, we need to make sure we got the best naming for the parameter, the enum and its values in view of the other parameters that apply to the children: AlignItems and Justify.

Also, we should consider LTR vs RTL. First and Last are culture specific, so in RTL mode First should stretch the right most child. And since we use Start and End for all other culture specific directions in the lib we should use these names instead.

Naming suggestion:

[Parameter]
public StretchItems StretchItems { get; set; }

public enum StretchItems {
  None,
  Start,
  End,
  StartAndEnd,
  Middle, 
  All,
}

Usage:

<MudStack StretchItems="StretchItems.End">
  ...
</MudStack>

It might be good to add a StartAndEnd also.

@ScarletKuro
Copy link
Member

ScarletKuro commented Apr 2, 2024

I like the functionality, it is very useful

Yeah, I think this is very common functionality in the Xamarin, Flutter, MAUI, WPF etc.

@BieleckiLtd
Copy link
Contributor Author

[...] we need to make sure we got the best naming for the parameter, the enum and its values in view of the other parameters [...]

Taking this further, perhaps StretchItems with Stretch enum would be more appropriate?

<MudStack AlignItems="Align.Start" StretchItems="Stretch.End">
...
</MudStack>

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

Oh sorry, you didn't make the changes yet. Make sure to use the [CascadingParameter] public bool RightToLeft {get; set;} to achive the correct order for Start and End

@BieleckiLtd
Copy link
Contributor Author

Oh sorry, you didn't make the changes yet. Make sure to use the [CascadingParameter] public bool RightToLeft {get; set;} to achive the correct order for Start and End

MudStack has Reverse parameter which does that. Should this be replaced with CascadingParameter?

@BieleckiLtd
Copy link
Contributor Author

This is a tricky one because RTL does not mean the same in MudStack. When the stack is vertical (column), Reverse means bottom-to-top, not right-to-left.

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

I see that mudstack already implements right to left logic. so I guess you won't have to? It will be automatically correct?

image

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

This is a tricky one because RTL does not mean the same in MudStack. When the stack is vertical (column), Reverse means bottom-to-top, not right-to-left.

Reverse is independent of RTL
image

Question is, what should be done with Stretch.Start and Stretch.End when Reverse is true? I don't know

@BieleckiLtd
Copy link
Contributor Author

Whatever it is now feels natural in both LTR and RTL when reversed, and when not, so I'd leave it as is. I have changed parameter name to StretchItems, Enum name to Stretch, added Stretch.StartAndEnd + unit, corrected some other incorrect descriptions in MudStack and updated examples to use new generic MudChipSet.

This feature provides the flexibility to specify which children should expand to fill additional space.
</Description>
</SectionHeader>
<SectionContent Block FullWidth Code="StackStretchChildrenExample" ShowCode="false" DarkenBackground>
<StackStretchChildrenExample />
<SectionContent Block FullWidth Code="StackStretchItemsExample" ShowCode="false" DarkenBackground>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use Code="@nameof(StackStretchItemsExample)" and you may also add nameofs in the other sections

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, sorted.

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

Perfect, looks good. If you will add the nameofs real quick I'll merge this today.

@henon henon changed the title MudStack: provide a convenient way to manage the size of Stack's children MudStack: Add parameter StretchItems to have certain children fill space Apr 2, 2024
@BieleckiLtd
Copy link
Contributor Author

BieleckiLtd commented Apr 2, 2024

AlignItems="Align.Start"

I have made a mistake here, it's AlignItems="AlignItems.Start". Should Stretch enum be StretchItems then?

@henon
Copy link
Collaborator

henon commented Apr 2, 2024

In theory there could be other uses of stretching, i.e. for images where you would need Stretch.Uniform, Stretch.X, Stretch.Y etc, so yes, it is the safest bet to rename the enum to StretchItems

@henon henon merged commit 7d80115 into MudBlazor:dev Apr 2, 2024
4 checks passed
@henon
Copy link
Collaborator

henon commented Apr 2, 2024

Thanks @BieleckiLtd

@BieleckiLtd BieleckiLtd deleted the feature/stack-manage-children branch April 2, 2024 15:55
danielchalmers pushed a commit to danielchalmers/MudBlazor that referenced this pull request Apr 2, 2024
…ace (MudBlazor#8545)

Co-authored-by: Pawel Bielecki <Pawel.Bielecki2@leoni.com>
biegehydra pushed a commit to biegehydra/MudBlazor that referenced this pull request Apr 26, 2024
…ace (MudBlazor#8545)

Co-authored-by: Pawel Bielecki <Pawel.Bielecki2@leoni.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack - Add option to control size (stretch) of children to fill main-axis
4 participants