[REVERTED] Fix 8503, 8787 - text in Entry not immediately visible, or visible after IsVisible set to true

[WayaFlyfeather]

Description of Change

Adding a ContentPresenter to the ScrollViewer in FormsTextBoxStyle, as setting VerticalAlignment on the ScrollViewer led to unexpected results.

Issues Resolved

• fixes (but reverted) [Bug] Text is not visible on Entry element after animation finished #8503
• fixes (but reverted) [Bug] Entry text initially invisible on UWP #8787

API Changes

None

Platforms Affected

• UWP

Behavioral/Visual Changes

Before this fix, if an Entry was changed from IsVisible = false to IsVisible true, text inside the entry would not actually become visible before it acquired focus.

[Edit January 30, 2020]: Other cases has popped up where entry text is seemingly randomly invisible at first. I have today confirmed that this fix also fixes issue #8787.

Before/After Screenshots

Issue 8503
Before 1 - before clicking button to make Entry visible

Before 2 - after clicking button to make Entry visible. Not, the text is not visible.

Before 3 - after clicking in the Entry, giving it focus - now showing text

After 1 - before clicking button to make Entry visible

After 2 - after clicking button to make entry visible. Note that the text is now visible

Issue 8787
Before fix 1 - Page is loaded, text in entry invisible:

Before fix 2 - after clicking in Entry to give it focus, text becomes visible;

After fix - page is loaded, text in Entry is immediately visible:

Testing Procedure

Issue 8503
Go to test page for Issue 8503 in the ControlGallery.
Click the button Click to show Entry for Isolated bug.
Check that the entry appears, and it's text content is visible.
If not, check if it appears when the entry receives focus. If it does, the bug is not fixed.

A similar test is repated below this, this one mimics the original problem described in #8503, it should similarly show the text at once when the button is clicked.

Issue 8787
Go to test page for Issue 8787 in the ControlGallery.
Check that the text in the yellow Entry is immediately visible.
If it is, the issue is fixed. If it is not visible before the Entry gains focus or the window is resized, the issue is not fixed.

Notes

This one is a bit weird.

I isolated the problem described in #8503, to occurring when an Entry started out as invisible, but was later changed to Visible. In that case it's text content would not be shown before it got focus. (The animation used in the original report changed visibility of hte item, which was the cause - it wasn't due to behaviors or animations).

I then determined that the "culprit" was this line introduced in #6463:
https://github.com/andreinitescu/Xamarin.Forms/blob/3481a842b45c463c1c0ff40c051e3c2484907c85/Xamarin.Forms.Platform.UAP/FormsTextBoxStyle.xaml#L224

If this VerticalAlignment was not present on the ScrollViewer, then everything worked fine. It also worked fine if I gave the Entry a HeightRequest, I discovered. But without a HeightRequest, the problem was there, even if changed the VerticalAlignment to "Center", i.e not binding it.

I could on Stack Overflow see that others have had problems with strange behavior from a ScrollViewer in a TextBox with VerticalAlignment.

Hence this proposed solution, instead og setting VerticalAlignment directly on the ScrollViewer, to have it wrap a ContentPresenter, and set VerticalContentAlignment on that instead.

It seems to be working fine, but to be perfectly honest I'm not 100% sure if I may have overlooked some potential problem with this solution.

PR Checklist

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

[WayaFlyfeather]

I have tested and confirmed that this PR also seems to fix #8787. I have updated this PR with a test page for that issue, and added screenshots.

[bmacombe]

@WayaFlyfeather you might want to test the issue 2172 page against this branch also. When I tested your changes with mine in #8214 the combo caused an issue at that time. Since #8214 is merged already just want to make sure there is no regression there.

Looking forward to getting this merged, I've seen some of the text visibility issues in UWP recently myself.

[WayaFlyfeather]

@bmacombe Right, will do, good thinking!

[bmacombe]

@WayaFlyfeather Great!, let me know if you need anything from me, glad to help.

[bmacombe]

@WayaFlyfeather If your fix takes care of the root issue behind the entry and editors awesome! Trying to figure out the root issue behind the incorrect sizing was just a dead end for me, so my fix was mainly a hack that was already approved by being in the code base already.

I remember the initial editor sizing method I borrowed and expanded upon was there to make sure an auto size editor would expand as it text wrapped. So probably want to make sure that scenario is tested.

I'll try to pull down your new branch and play with it some also in the next couple days.

[WayaFlyfeather]

@bmacombe Thanks, Brian, that would be great :)

The autosizing Editor in page 2172 does seem to work, actually - at least at first glance.

I'm not sure exactly what is happening, either - but it does seem that it is somewhat more healthy to include a ContentPresenter in a ScrollViewer, instead of treating the ScrollViewer as the ContentPresenter.

[WayaFlyfeather]

@bmacombe So, it seems this fix is not a miracle solution after all. I looked at the original PR for the AutoSize in the Editor, and could see that one of the problems occuring was when rapidly changing the size, such as when holding down the enter or back space keys.

The Editor with AutoSize in the 2172 test page does seem to handle holding down Enter ok; unfortunately it seems when holding down back space some lines are not visually deleted, until next time the content is changed.

So I guess some solution will have to be found; either making the measuring code work with this fix, or find some other work around. Or perhaps make two styles for the FormsTextBox, one that is for multi line content and one for single lines.

Will see if I can come up with something :)

[bmacombe]

@WayaFlyfeather If nothing else, maybe the measure method could be modified to remove ContentElement from the control template for the static text box when it's created. Saves having two control templates in case other changes are made in the future to it.

Example: _rootGrid = (Windows.UI.Xaml.Controls.Grid)GetTemplateChild("RootGrid");

Seems a little hacky to me, but that whole measure using a static text box outside the visual tree feels like a hack too...so 🤷‍♂️

Would just need to assign a name to the ScrollViewer and then use GetTemplateChild to access it. Then just remove it's content.

This could be in done in FormsTextBox.cs in the GetCopyOfSize in this if statement.

if (_copyOfTextBox == null)
			{
				_copyOfTextBox = new FormsTextBox
				{
					Style = Windows.UI.Xaml.Application.Current.Resources["FormsTextBoxStyle"] as Windows.UI.Xaml.Style
				};

				// This causes the copy to be initially setup correctly. 
				// I found that if the first measure of this copy occurs with Text then it will just keep defaulting to a measure with no text.
				_copyOfTextBox.Measure(_zeroSize);
			}

I'm going to be on a business trip the next few days and won't have a chance to dig more. Hopefully this gives you something to think about, let me know if you have any questions.

[samhouts]

Off topic comment....this collaboration is why I love this community so much! Thank you, @WayaFlyfeather and @bmacombe !!

[PureWeen]

Build failure was just the OSX box timing out

[bmacombe]

@PureWeen I just pulled down 4.5 after you merged this PR. It regresses the fix in #8214 still.

the changes in #8214 could be undone but then the editor auto size is broken

You can see all of this in the #2172 issue page.

[PureWeen]

@bmacombe my fault I had missed the discussion on this one

Let me just revert this and reopen

[PureWeen]

alright I've reverted this PR

#9598

Once the other highlighted issues are addressed create a new PR

Thank you for the watchful eye on this one @bmacombe !!

[bmacombe]

@PureWeen You're welcome!

@WayaFlyfeather Have you had a chance to dig into this more? I might be able to find some time this weekend or next week.

[WayaFlyfeather]

@PureWeen You're welcome!

@WayaFlyfeather Have you had a chance to dig into this more? I might be able to find some time this weekend or next week.

@bmacombe Sorry, got suddenly very busy & caught up in some RL stuff. Will try to get back to this over the weekend.

@PureWeen Sorry for the trouble!

And an off-topic thank you, @samhouts! :)