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

Update CornerRadius property on NavigationViewItem, PivotHeaderItem and ScrollBar #922

Merged
merged 13 commits into from
Jul 3, 2019
Merged

Update CornerRadius property on NavigationViewItem, PivotHeaderItem and ScrollBar #922

merged 13 commits into from
Jul 3, 2019

Conversation

kaiguo
Copy link
Contributor

@kaiguo kaiguo commented Jun 22, 2019 •
edited
Loading

Update following controls to make sure CornerRadius property does the right thing.

  • NavigationViewItem
  • PivotHeaderItem
  • ScrollViewer
  • ScrollBar

@kaiguo kaiguo requested a review from chrisglein June 22, 2019 00:47
@kaiguo kaiguo requested a review from a team as a code owner June 22, 2019 00:47
@kaiguo kaiguo self-assigned this Jun 22, 2019
kaiguo
kaiguo commented Jun 22, 2019
View reviewed changes
Copy link
Contributor Author

@kaiguo kaiguo left a comment

Choose a reason for hiding this comment

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

ScrollBar template is moved over, corner radius changes are in this commit: 4b6da9c

chrisglein
chrisglein requested changes Jun 22, 2019
View reviewed changes
Copy link
Member

@chrisglein chrisglein left a comment

Choose a reason for hiding this comment

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

Can you bring over the ScrollBar templates without edits first (to account for the squash merge that will happen)?

dev/dll/Microsoft.UI.Xaml.vcxproj Outdated Show resolved Hide resolved
dev/ScrollBar/ScrollBar_rs4_themeresources.xaml Show resolved Hide resolved
@kaiguo kaiguo force-pushed the user/kaiguo/corner-radius branch from 4b6da9c to 4d88ffe Compare June 22, 2019 00:54
@kaiguo kaiguo force-pushed the user/kaiguo/corner-radius branch from 4d88ffe to eff2adb Compare June 22, 2019 00:55
@kmahone
Copy link
Member

kmahone commented Jun 24, 2019

Can we add test coverage for these changes? Even if the test is just to navigate to a Page the contains these controls to ensure that the template can be applied without crashing. Or do we already have such a test?

kmahone
kmahone requested changes Jun 24, 2019
View reviewed changes
Copy link
Member

@kmahone kmahone left a comment

Choose a reason for hiding this comment

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

I wish I could set "Request Changes" status on a GitHub review without having to leave an extra comment.

@kaiguo kaiguo requested a review from kmahone June 26, 2019 19:46
@jevansaks jevansaks added the release note PR that we want to call out in the next release summary label Jun 28, 2019
@kaiguo kaiguo added the auto merge This PR will be merged once all checks pass label Jul 3, 2019
@kaiguo kaiguo merged commit 7897ef3 into master Jul 3, 2019
@kaiguo kaiguo deleted the user/kaiguo/corner-radius branch July 3, 2019 17:27
@msft-github-bot
Copy link
Collaborator

🎉Microsoft.UI.Xaml v2.2.190731001-prerelease has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@licanhua licanhua licanhua left review comments

@kmahone kmahone kmahone approved these changes

@chrisglein chrisglein chrisglein approved these changes

Assignees

@kaiguo kaiguo

Labels
area-Styling auto merge This PR will be merged once all checks pass release note PR that we want to call out in the next release summary
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kaiguo @kmahone @msft-github-bot @licanhua @chrisglein @jevansaks