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

[C] add null check for FlexLayout.OnPropChanged #5012

Merged
merged 1 commit into from
Jan 18, 2019
Merged

Conversation

StephaneDelcroix
Copy link
Member

@StephaneDelcroix StephaneDelcroix commented Jan 18, 2019

Description of Change

We had null checks in 3 places, not the fourth one. and that case was
hit by #5001.

Issues Resolved

API Changes

None

Platforms Affected

  • Core (all platforms)

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

PR Checklist

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

We had null checks in 3 places, not the fourth one. and that case was
hit by #5001.

- fixes #5001
@@ -344,6 +344,8 @@ void OnChildPropertyChanged(object sender, PropertyChangedEventArgs e)
if ( e.PropertyName == WidthRequestProperty.PropertyName
|| e.PropertyName == HeightRequestProperty.PropertyName) {
var item = (sender as FlexLayout)?._root ?? GetFlexItem((BindableObject)sender);
if (item == null)
Copy link
Member Author

Choose a reason for hiding this comment

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

this code is repeated 4 times, but can't be moved a level higher, as it would imply retrieving the item for all possible property changed.

@StephaneDelcroix StephaneDelcroix merged commit fd11b9f into 3.5.0 Jan 18, 2019
@StephaneDelcroix StephaneDelcroix deleted the fix_gh5001 branch January 18, 2019 12:42
Liam2349 added a commit to Liam2349/Xamarin.Forms that referenced this pull request Jan 19, 2019
…t, (xamarin#5012), my previous solution did not consider that the method is called for more than just the stated cases, and that the "item" retrieval and null check are not needed every time this method is hit.
@samhouts samhouts added this to the 3.5.0 milestone Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants