Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

[Core] Fix layout padding update issue#4166

Merged
StephaneDelcroix merged 1 commit intoxamarin:3.4.0from
myroot:fix-layout-padding-issue
Oct 26, 2018
Merged

[Core] Fix layout padding update issue#4166
StephaneDelcroix merged 1 commit intoxamarin:3.4.0from
myroot:fix-layout-padding-issue

Conversation

@myroot
Copy link
Copy Markdown
Contributor

@myroot myroot commented Oct 21, 2018

Description of Change

This PR update code of IPaddingElement.OnPaddingPropertyChanged on Layout
from InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged) to InvalidateLayout()

Because, When size of layout was not changed by Padding, children element does not re-layouting with InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged)

Issues Resolved

API Changes

None

Platforms Affected

  • Core/XAML (all platforms)

Behavioral/Visual Changes

None

Before/After Screenshots

  • Before
    before

  • After
    after

Testing Procedure

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@myroot myroot force-pushed the fix-layout-padding-issue branch from 70941b0 to c2249b4 Compare October 21, 2018 23:21
Copy link
Copy Markdown
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

The gallery page is nice, but adding a UnitTest (in X.F.Core.UnitTests) will ensure that we do not regress this in the future. Do you think you could add one ?

@myroot myroot force-pushed the fix-layout-padding-issue branch from c2249b4 to 2991788 Compare October 23, 2018 03:24
@myroot
Copy link
Copy Markdown
Contributor Author

myroot commented Oct 23, 2018

@StephaneDelcroix Sure, I update UnitTest(StackLayoutUnitTests)

@StephaneDelcroix
Copy link
Copy Markdown
Member

@myroot one last thing. could you rebase this on 3.4.0 ?

@myroot
Copy link
Copy Markdown
Contributor Author

myroot commented Oct 24, 2018

@StephaneDelcroix Ok, but I can update on friday. and Should I pull request into 3.4.0 branch?

@PureWeen PureWeen added the approved Has two approvals, no pending reviews, and no changes requested label Oct 24, 2018
@PureWeen
Copy link
Copy Markdown
Contributor

PureWeen commented Oct 24, 2018

@myroot you can rebase the branch by

  • git rebase --onto 3.4.0 master fix-layout-padding-issue
  • fix any conflicts
  • git push origin fix-layout-padding-issue --force

Once you've done that then

  • come back to this PR
  • click EDIT (at the top)
  • change the base branch

@PureWeen PureWeen added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Oct 24, 2018
 - Update PaddingPropertyChanged handler
 - Add UnitTest for Padding
 - Update ControlGallery
@myroot myroot force-pushed the fix-layout-padding-issue branch from 2991788 to e017c22 Compare October 25, 2018 22:55
@myroot myroot changed the base branch from master to 3.4.0 October 25, 2018 22:55
@myroot
Copy link
Copy Markdown
Contributor Author

myroot commented Oct 25, 2018

@PureWeen Thanks!
@StephaneDelcroix I rebase on 3.4.0

@PureWeen
Copy link
Copy Markdown
Contributor

@myroot Thank you!! I kicked off the UI tests again just to make sure we're all good. After that we'll merge

@StephaneDelcroix StephaneDelcroix merged commit 7ffa130 into xamarin:3.4.0 Oct 26, 2018
StephaneDelcroix pushed a commit that referenced this pull request Oct 26, 2018
- Update PaddingPropertyChanged handler
 - Add UnitTest for Padding
 - Update ControlGallery

- fixes #4165
@samhouts samhouts added the e/4 🕓 4 label Nov 2, 2018
@PureWeen PureWeen added this to the 3.4.0 milestone Nov 8, 2018
@samhouts samhouts removed retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Nov 26, 2018
@myroot myroot deleted the fix-layout-padding-issue branch November 27, 2019 05:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a/layout approved Has two approvals, no pending reviews, and no changes requested e/4 🕓 4 hacktoberfest 🍻 t/bug 🐛

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants