Orientation option for Segmented

[Avid29]

Fixes #746

PR Type

What kind of change does this PR introduce?

What is the current behavior?

There is not orientation options for the Segmented control

What is the new behavior?

Add customization options for a vertical Segmented control

PR Checklist

Please check if your PR fulfills the following requirements:

• Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
• Based off latest main branch of toolkit
• Tested code with current supported SDKs
• Tests for the changes have been added (if applicable)
• Header has been added to all new source files
• Contains NO breaking changes

Other information

[Avid29]

So far I've added an Orientation property to the EqualPanel. I tested it (see below), but it's not actually implemented anywhere yet.

[Avid29]

Here's an update.

Though this does have a slight change in the horizontal style, namely the selected indicator is centered on the full SegmentedItem rather than just the content. Here's what that difference looks like in practice.

Before:

After:

[michael-hawker]

@niels9001 was there a reason the indicator was centered under the content only vs. the entire content? That'll only matter in Icon + Text Horizontal mode.

[michael-hawker]

@Avid29 this works perfect!

Just slotted right into my VSM with changing the orientation and I was done! 🦙❤️ I'll take a look through the code itself in a bit.

[michael-hawker]

Looks pretty clean to me. @niels9001 @Arlodotexe let me know your thoughts.

[Avid29]

This is now blocking for #752

[niels9001]

Horizontal center alignment of the pill under the label is per design spec. But I'm fine with this change as this would align it with NavigationView too.

[Avid29]

@niels9001 I believe I could get the pill back under the content by finagling the column layout and visual state setters, but as is now I believe it makes the default style's layout more predictable. It might even be simpler if I adjust the default VisualState (though they're always set, so it wouldn't change any behavior).

[niels9001]

This is looking great - and will be really helpful in creating selectable cards or things like a Store-like NavView :)!

Three things that come to mind:

1. Long text labels don't fit.. Maybe we should use texttrimming for this so it just ... for the chars that no longer fit?

2. It feels like we need a bit more spacing vertically (maybe 4 instead of 1) by default to better differentiate what icon belongs to what label. Should we introduce a HorizontalSegmentedItemSpacing and VerticalSegmentedItemSpacing?

3. Have we tested the vertical orientation with the built-in styles too? Could we add an orientation dropdown to this sample too so it's easier to validate any changes in the future?

[Avid29]

@niels9001 I believe I could get the pill back under the content by finagling the column layout and visual state setters, but as is now I believe it makes the default style's layout more predictable. It might even be simpler if I adjust the default VisualState (though they're always set, so it wouldn't change any behavior).

I looked into it. I'd forgotten that to get the pill to left align properly I had to move it outside of the ContentHolder. This makes it much much harder, since it's no longer a child of the grid separating the icon from the content.

[niels9001]

This also happened.. I think it's expected, and probably up to the developer to not use Stretch in vertical mode, right?

[Avid29]

Hmm. Yeah, idk what expected behavior is there. You're definitely right that I should add options to more of the samples though.

[Avid29]

3. Have we tested the vertical orientation with the built-in styles too? Could we add an orientation dropdown to this sample too so it's easier to validate any changes in the future?

Yes, I did think about this. I explcitly decided that the PivotSegmentedStyle ignores the property, though to be honest it would be quite harmless to make it stack, same as ButtonSegmentedStyle. Here's a screenshot with that sample option added.

[michael-hawker]

Sorry @Avid29 I should have merged maybe for the main update vs. rebase as you're working on this. Wanted to pick up the fixes for the tests to be more reliable for the CI.

[Avid29]

No problem 👍

[michael-hawker]

Held up by #758

[michael-hawker]

Hopefully CI issues are resolved now. Think there's just a few small comments left and for @niels9001 to approve if the new style changes look good?

[Avid29]

Looks like the CI is fine. The issue is just using ref fields in UWP. Hmm...

[niels9001]

The visual changes look great to me :)! Thanks a lot @Avid29 !

Stale

[lgztx96]

I noticed that the PR adding Orientation support to Segmented has already been merged, but I encountered issues during testing while porting this to the C++ version.

In vertical mode, the pill should be positioned on the left, but it is currently rendered below. Because the Orientation property is changed during initialization before the item is added to the container, Item state updates rely on the Orientation property change callback, so dynamically binding the ItemSource may also result in incorrect behavior.

This can be reproduced simply by setting the following XAML:

 <controls:Segmented Orientation="Vertical">
     <controls:SegmentedItem Content="Item 1" />
     <controls:SegmentedItem Content="Item 2" />
     <controls:SegmentedItem Content="Item 3" />
     <controls:SegmentedItem Content="Item 4" />
 </controls:Segmented>

Alternatively, set the default value of Orientation to Vertical in the sample application.

[Avid29]

@lgztx96 Good catch! On it.