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

Bug: CornerRadius Inconsistency/Missing in ContentDialog/MessageDialog #1345

Closed
robloo opened this issue Sep 17, 2019 · 16 comments · Fixed by #1405
Closed

Bug: CornerRadius Inconsistency/Missing in ContentDialog/MessageDialog #1345

robloo opened this issue Sep 17, 2019 · 16 comments · Fixed by #1405
Assignees
Labels
area-UIDesign UI Design, styling bug Something isn't working team-Controls Issue for the Controls team

Comments

@robloo
Copy link
Contributor

robloo commented Sep 17, 2019

Describe the bug
There is some inconsistency with the added corner radius in WinUI 2.2 styling (#659). The buttons in ContentDialog did not get a corner radius in all cases. Other dialog types probably have the same issue (MessageDialog).

Screenshots

(1) As you can see the button on the right has the radius, the button on the left does not.

Capture1

Capture3

Additonal Questions:
(2) When an item has keyboard focus like the buttons in the above image, should the added context border have a corner radius as well?
Edit: As answered below this is by design...

(3) AppBarButton doesn't have a CornerRadius. I actually think this looks better and should stay that way. However, was this and related AppBar controls missed in the discussion?
Edit: As answered below this is by design.

Capture2

Version Info

Microsoft.UI.Xaml 2.2.190830001

Windows 10 version Saw the problem?
May 2019 Update (18362) Yes
October 2018 Update (17763) N/A
April 2018 Update (17134) N/A
Fall Creators Update (16299) N/A
Creators Update (15063) N/A
Device form factor Saw the problem?
Desktop Yes
Mobile N/A
Xbox N/A
Surface Hub N/A
IoT N/A
@jevansaks
Copy link
Member

Buttons in ContentDialog is probably an oversight. Buttons in MessageDialog is owned by Windows and would be part of a future OS update.

AppBarButton ... i'm not sure. @chigy?

@jevansaks jevansaks added the area-UIDesign UI Design, styling label Sep 17, 2019
@chigy
Copy link
Member

chigy commented Sep 17, 2019

@robloo , @jevansaks
1 - A bug. All buttons are supposed to get rounded.
2 - Don't understand the question. If the question is if the keyboard focus should be rounded, then the answer is no. Not part of today's design.
3 - It is by design per "T-rule" when multiple items are next to each other and forms a T, it should not be rounded. Thus, we don't round list items nor grid items.

@robloo
Copy link
Contributor Author

robloo commented Sep 17, 2019

@chigy

2 - Don't understand the question. If the question is if the keyboard focus should be rounded, then the answer is no. Not part of today's design.

Yes, that is my question (should have said 'Focus'). Further annotated below:

Capture3

3 - It is by design per "T-rule" when multiple items are next to each other and forms a T, it should not be rounded. Thus, we don't round list items nor grid items.

Makes a lot of sense and definitely visually it's the right call. Can this be added to docs some place so developers like myself are aware. We are going to have to know the design conventions with CornerRadius.

Probably here: https://docs.microsoft.com/en-us/windows/uwp/design/style/

I can add feedback to the document on GitHub if it will help.

@chigy
Copy link
Member

chigy commented Sep 17, 2019

@robloo , Yes, it is by design that we don't round and yes we plan on adding some note to that since as a developer you can round the corner as much as you like but they might not look good. Detail is still being worked out as to what to say and where, but early draft is here. https://github.com/microsoft/microsoft-ui-xaml-specs/blob/user/chigy/roundedcorner/active/RoundedCorner/roundedcorner.md

@chigy
Copy link
Member

chigy commented Sep 17, 2019

@robloo , can you please open a bug against # 1 so I can close this issue as resolved since the other two issues are actually by current design?

@robloo
Copy link
Contributor Author

robloo commented Sep 17, 2019

@chigy I will re-title this one. No need to make a new bug report.

@chigy chigy removed their assignment Sep 17, 2019
@chigy
Copy link
Member

chigy commented Sep 17, 2019

Removing myself so it will appear at triage. Please consider fixing # 1 issue. FYI @jevansaks

@robloo robloo changed the title Bug: CornerRadius Inconsistency/Missing on some Controls Bug: CornerRadius Inconsistency/Missing in ContentDialog/MessageDialog Sep 17, 2019
@jevansaks jevansaks added the bug Something isn't working label Sep 17, 2019
@jevansaks
Copy link
Member

Thanks @robloo! @kaiguo, FYI.

@ismaelestalayo
Copy link

Apart from the buttons that are not in the Selected state not having the CornerRadius set, the ContentDialog itself doesn't have it either:

ContentDialog

@chigy
Copy link
Member

chigy commented Sep 23, 2019

@kaiguo , ContentDialog not having corner working was something to do with a bug that you were working through? Though I thought we shipped with corner rounded but the bug was about customization?

@robloo
Copy link
Contributor Author

robloo commented Dec 15, 2019

@kaiguo @chigy Are the buttons in the ContentDialog supposed to have a border brush? This doesn't match with the default button style.

pic1
Microsoft.UI.Xaml 2.3.191211002

This came up investigating #1764

@chigy
Copy link
Member

chigy commented Dec 16, 2019

@robloo , which version are you using because I am seeing the right behavior on my older Controls gallery (this one doesn't have rounded corner fixed) so did something get reverted when others got fixed?

image

@msft-github-bot msft-github-bot removed the needs-triage Issue needs to be triaged by the area owners label Dec 16, 2019
@marcelwgn
Copy link
Contributor

I am also seeing the right behavior on the latest commit on master of this repository. Following screenshot was taken inside the MUXControlsTestApp:

image

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Dec 16, 2019
@YuliKl YuliKl removed the needs-triage Issue needs to be triaged by the area owners label Dec 16, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 17, 2019

@chigy @chingucoding Ok, thanks for taking a look. This is a strange state my app is getting into then and it looks like it's tied with #1764. Since the style is correct for you, I won't worry about it here.

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Dec 17, 2019
@StephenLPeters StephenLPeters removed the needs-triage Issue needs to be triaged by the area owners label Dec 19, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 23, 2019

Update: The border on the buttons in the content dialog was a bug on my part. Sorry for that!

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Dec 23, 2019
@kaiguo
Copy link
Contributor

kaiguo commented Dec 23, 2019

Update: The border on the buttons in the content dialog was a bug on my part. Sorry for that!

Thanks for letting us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-UIDesign UI Design, styling bug Something isn't working team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants