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

CornerRadius related APIs #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kaiguo
Copy link

@kaiguo kaiguo commented Aug 14, 2019

No description provided.

* Add CornerRadiusFilterConverter api doc

* Apply suggestions from code review

Co-Authored-By: Yulia Klein <yulikl@microsoft.com>

* Spec formatting/info updates (#46)

More background info, reorder sections, use ControlTemplate for the example code.

* Added enum property

* Add attached property spec

* cr feedback

* Combined specs and added some background
@kaiguo kaiguo requested a review from a team as a code owner August 14, 2019 00:16

But there are cases where the ControlTemplate needs to get only some of the components out of the CornerRadius struct, for example the top left and right corners, in order to have a rounded top and a square bottom. There's no way to create a TemplateBinding to a struct field, so you can't bind to CornerRadius.TopLeft.

For example when CommandBarFlyout opens up its secondary menu, primary and secondary menus are clipped together we need to clear rounded corners on bottom of primary menu and top of secondary menu to close the gaps. The following shows circles on the rounded corners and squares on the squared corners:
Copy link
Member

Choose a reason for hiding this comment

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

Lots of inside baseball in this paragraph. I think this translates into

For example, when the CommandBarFlyout opens up its secondary menu, the secondary menu is placed adjacent to the primary menu. To make the secondary menu appear to be an extension of the primary menu, we need to remove the rounded corners from the bottom of the primary menu and the top of the secondary menu (green squares). On the other hand, the top corners of the primary menu and bottom corners of the secondary menu remain rounded (red circles).


![CommandBarFlyout CornerRadius example](images/commandbarflyout-cornerradius-example.png)

The solution in this spec is a stock IValueConverter that can be used with the TemplateBinding to extract fields out of the struct.
Copy link
Member

Choose a reason for hiding this comment

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

"extract fields out of the struct" is confusing, because it sounds like it's a converter that takes a CornerRadius and returns the value of a specific field. Perhaps

This spec provides a CornerRadiusFilterConverter which retains the corner radius on selected corners, while converting the other corners to square corners. For example, setting Filter="Top" causes the CornerRadiusFilter to preserve the corner radius for the TopLeft and TopRight corners, while setting the BottomLeft and BottomRight corner radii to zero, thereby making them square.

# Description

## CornerRadiusFilterConverter class
Use the CornerRadiusFilterConverter with a Binding or TemplateBinding to create a new CornerRadius struct from an existing one, extracting only some of the fields, leaving the others zero.
Copy link
Member

Choose a reason for hiding this comment

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

I think "extracting" should be "preserving". The word "extracting" suggests that the value is "extracted from" the source, and treated as the extracted value. E.g.

Height="{Binding CornerRadius, Converter={StaticResource TopLeftCornerFilter}}"

This takes the CornerRadius, extracts the top left corner (producing a Double) and sets that as the Height.

But that's not what this does. This takes the extracted values, then re-creates a new CornerRadius from them.

Use the CornerRadiusFilterConverter with a Binding or TemplateBinding to create a new CornerRadius struct from an existing one, extracting only some of the fields, leaving the others zero.

## AutoSuggestBoxHelper/ComboBoxHelper KeepInteriorCornersSquare property
Set this property on an AutoSuggestBox/ComboBox to keep the interior corners square, even if the CornerRadius property is non zero.
Copy link
Member

Choose a reason for hiding this comment

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

Took me a while to figure out that the "interior corners" are the bottom corners of the fixed portion + the top corners of the dropdown portion. May want state this explicitly.

If the AutoSuggestBox/ComboBox KeepInteriorCornersSquare property is True, then the bottom corners of the fixed portion of the control and the top corners of the drop-down list are made square, even if the CornerRadius property is nonzero.

Top,
Right,
Left,
Bottom
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this be better as

[webhosthidden]
[flags]
public enum CornerRadiusFilterCorners
{
    TopLeft = 0x0001,
    TopRight = 0x0002,
    BottomLeft = 0x0004,
    BottomRight = 0x0008,
    Top = TopLeft | TopRight,
    Left = TopLeft | BottomLeft,
    Bottom = BottomLeft | BottomRight,
    Right = TopRight | BottomRight,
}

That would let met write not only

<CornerRadiusFilterConverter Corners="Top" x:Name="TopCornerFilter" />

but also

 <CornerRadiusFilterConverter Corners="TopLeft,TopRight,BottomRight" x:Name="CartoonBalloonCornerFilter" />
 <CornerRadiusFilterConverter Corners="TopLeft,BottomRight" x:Name="TiaaLogoCornerFilter" />

TIAA logo

Copy link
Author

Choose a reason for hiding this comment

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

We saw some issues that values like "TopLeft,TopRight,BottomRight" are not evaluated correctly in CX app. (See this comment microsoft/microsoft-ui-xaml#1252 (comment).) So we can't really do this right now. I've filed a bug on markup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants