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

Add logic to set DefaultLabelPosition on the WinUI CommandBar #15835

Merged
merged 5 commits into from Jul 13, 2023
Merged

Add logic to set DefaultLabelPosition on the WinUI CommandBar #15835

merged 5 commits into from Jul 13, 2023

Conversation

Foda
Copy link
Member

@Foda Foda commented Jun 23, 2023

Description of Change

Make the default label position on WinUI CommandBar use "right" by default, or if there's only icons collapsed by default.

Issues Fixed

Fixes #12063

PureWeen
PureWeen requested changes Jun 23, 2023
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

Can we add tests?

If you look in ControlsHandlerTestBase.Windows.cs you'll see some helper methods in there from grabbing the command bar so you can validate the different pivots.

mattleibow
mattleibow reviewed Jun 27, 2023
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This looks great!

can you add a test to check when adding text that it causes the layout to change. For example, add items with just icons and display, the change to have text and check to see it updates to Right.

@Foda
Copy link
Member Author

Foda commented Jun 27, 2023

This looks great!

can you add a test to check when adding text that it causes the layout to change. For example, add items with just icons and display, the change to have text and check to see it updates to Right.

Ah, that's a good idea.

@PureWeen PureWeen self-requested a review June 27, 2023 18:45
@rachelkang rachelkang mentioned this pull request Jun 28, 2023
Closed
@PureWeen PureWeen merged commit 43b9397 into main Jul 13, 2023
28 checks passed
@PureWeen PureWeen deleted the foda/WinCmdBarLabelPos branch July 13, 2023 08:54
MartyIX
MartyIX reviewed Jul 13, 2023
View reviewed changes
src/Controls/tests/DeviceTests/Elements/Toolbar/ToolbarTests.Windows.cs

var toolbar = (Toolbar)(navPage.Window as IToolbarElement).Toolbar;
var platformCommandBar = ((MauiToolbar)toolbar.Handler.PlatformView).CommandBar;
Assert.True(platformCommandBar.DefaultLabelPosition == CommandBarDefaultLabelPosition.Right);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not Assert.Equal?

@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MartyIX MartyIX MartyIX left review comments

@mattleibow mattleibow mattleibow left review comments

@PureWeen PureWeen PureWeen approved these changes

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

Assignees
No one assigned
Labels
area/toolbar platform/windows 🪟
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[WinUI] Add logic to set DefaultLabelPosition on the WinUI CommandBar
5 participants
@Foda @MartyIX @mattleibow @PureWeen @Eilon