[UWP] Attempt to resolve entry on UWP not correctly calculating the correct height when in a scroll view

[bmacombe]

Description of Change

It seems that when the UWP textbox is in a visual tree that includes a scroll view it seems to mis-measures the height of the text box. This change will set the native text box text property to string.empty while measuring which seems to result in the correct height being calculated. Once that is calculated, it sets the text value back from the XF element.

Used the method of using a static TextBox to do the measure that was already in use in the EditorRenderer to handle autosize. I pulled that code out and modified it, adding it as a static method on FormsTextBox. I then updated the EntryRenderer and EditorRenderer to use this code for measuring their size which corrects the issue.

To help validate I recreated the previous behavior in custom renderers (in the UWP Control Gallery) to show the previous behavior and the new behavior side by side on the issue page.

Issues Resolved

• fixes Height of Entry with data binding incorrect on UWP when Entry in ScrollView in Grid #2172
• fixes Layout issue with Entry #5711

API Changes

None

Platforms Affected

• UWP

Behavioral/Visual Changes

Correctly calculates the height of a text box in a scroll view.

Before/After Screenshots

Top entry using the existing renderer
Buttom entry using the experimental renderer

Testing Procedure

Open the issue page and view the entries

PR Checklist

• Targets the correct branch
• Tests are passing (or failures are unrelated)

[bmacombe]

This PR needs to be finalized, looking for feedback from the XF team and the community before making final changes.

[bmacombe]

This solution likely won't work, after more review I see unintended events being fired by changing the text value. The EntryRenderer updates the XF Entry when the native Textbox text changes.

[bmacombe]

I wonder if this issue is related to this comment in VisualElementRenderer.

// Disabled until reason for crashes with unhandled exceptions is discovered
// Without this some layouts may end up with improper sizes, however their children
// will position correctly
//Loaded += (sender, args) => {
if (Packager != null)
	Packager.Load();
//};

[bmacombe]

It does appear to be related. Calling Packager.Load() in the Loaded event in SetElement line 150 in visual state renderer resolves the issue. But that comment appears to be from Jason Smith. Thoughts from the XF team with more knowledge on how this process works would be appreciated.

// Disabled until reason for crashes with unhandled exceptions is discovered
// Without this some layouts may end up with improper sizes, however their children
// will position correctly
Loaded += (sender, args) =>
{
	if (Packager != null)
		Packager.Load();
};

[bmacombe]

Lastest observation. I tried stepping through the layout logic and seeing if I could force the items hosted by the ScrollViewer to update their layout, but I noticed that all the UWP LayoutUpdate, Loaded, etc events were being called before the first the requested size of the entry, so they had no effect.

I then refocused on the GetDesiredSize method of VisualElementRender and after just some random experimentation I noticed that heightConstraint was double.Infinity, which makes sense inside a ScrollViewer. But I don't think the UWP layout logic likes this.

Changing the heightConstraint to 500,000 (still effectively infinity as far as layout goes), caused the correct height of the FormsTextBox to be calculated.

if (double.IsInfinity(heightConstraint))
	heightConstraint = 500000;
return base.GetDesiredSize(widthConstraint, heightConstraint);

This "Test Renderer" entry is using a renderer that contains the code above in it's GetDesiredSize override.

@samhouts Would it be possible to get a review on method? This seems to be a reasonable solution as I think it would have little side effects.

I did try to recreate this issue in a standard UWP app, but I haven't been able to. So it's still possible there is a XF layout logic issue I'm missing.

[bmacombe]

Well, scratch that one. It works in the simple layout in the Issue page, but not in a more complex layout in my own app. Hopefully these notes will help spur some thought in others.

[bmacombe]

Another note, I see the UWP TextBox control template also includes a ScrollViewer. Maybe an issue?

<ScrollViewer x:Name="ContentElement" AutomationProperties.AccessibilityView="Raw" Grid.Column="0" HorizontalScrollBarVisibility="{TemplateBinding ScrollViewer.HorizontalScrollBarVisibility}" HorizontalScrollMode="{TemplateBinding ScrollViewer.HorizontalScrollMode}" IsDeferredScrollingEnabled="{TemplateBinding ScrollViewer.IsDeferredScrollingEnabled}" IsHorizontalRailEnabled="{TemplateBinding ScrollViewer.IsHorizontalRailEnabled}" IsTabStop="False" IsVerticalRailEnabled="{TemplateBinding ScrollViewer.IsVerticalRailEnabled}" Margin="{TemplateBinding BorderThickness}" Padding="{TemplateBinding Padding}" Grid.Row="1" VerticalScrollMode="{TemplateBinding ScrollViewer.VerticalScrollMode}" VerticalScrollBarVisibility="{TemplateBinding ScrollViewer.VerticalScrollBarVisibility}" ZoomMode="Disabled"/>

[jfversluis]

Hi @bmacombe thank you for your ongoing, extensive research and writeup on this. We really appreciate it!

I see you asking some feedback and things from time to time, but it's hard to make it concrete since you are progressing on your own 😄

The only thing I can see is to not pay too much attention to the comments in there. They might be pretty old and not relevant anymore. Just trust your own judgement on how to solve this best and of course keeping in mind that other controls and scenarios should just remain working. Did you by any chance get the UI tests running for UWP? That might be a good indication to go on.

Is there anything concrete you want us to look at at this time?

[bmacombe]

@jfversluis I think you gave me what I need, Thanks!...I think the comment just made me nervous and unfortunately predates the import into GitHub so I haven't been able to track it back to an issue or find something in Bugzilla.

I think the right solution is to have the Packager.Load happen after the layout control has been loaded. I really think the issue is timing of when the various native UWP elements are created and asked to measure their size. Everything else seems way to much of hack...I might be willing to risk that in some app code...but not in framework code.

So I'll put Packager.Load back into the Loaded event and look into running the UWP UI tests. Just makes me nervous messing with such a root level item in the UWP platform. 😅

[jfversluis]

Sure, always be careful at this level, but that just means you have to test all the things impacted by it more carefully 😉

[jfversluis]

I do would like to get @PureWeen to either run the UWP UI tests or explain to @bmacombe how to run them :)

I think there are some instructions on the repo, did you see those? Here: https://github.com/xamarin/Xamarin.Forms/#run-uwp-ui-tests

[bmacombe]

I know how to run them 😁 but they don't run right on my machine...tried various versions of the driver etc. Not all the text gets filled in the various search boxes to pull up the tests. I had a little better luck if I changed the control gallery search bars to text boxes...but still had issues.

If anyone has some other suggestions to try I'd be glad to try them again.

[PureWeen]

@jfversluis running now

[PureWeen]

@jfversluis This doesn't appear to break anything with the unit tests

Is the removal of this file intentional
https://github.com/xamarin/Xamarin.Forms/pull/8214/files#diff-9d8f28a74e9d78f5c7af9201bf8f98bbL1

[bmacombe]

@PureWeen I removed it because I accidentally added (or thought I did) when I accidentally staged and checked in those resharper settings. If there was already one there, I didn't realize it.

[bmacombe]

I'll try to reverse that...new git skill to learn 😁

[bmacombe]

@PureWeen @jfversluis I believe I have reversed my resharper file delete...sorry about that!

[jfversluis]

It appears so, great! Thanks!