[UWP] Fixes 8787 Entry text not visible until focus or window resize

[bmacombe]

Description of Change

Updates VisualElementRenderer to delay calling Packager.Load until the Loaded event fires.

PR #11350 should be merged first

I'm not sure I completely understand the layout pipeline of the UWP backend, so I'm not 100% sure of all the possible effects of this change could have. It should be noted that the code to call Packager.Load in the Loaded event was previously commented out for causing an unhandled exception. I have not experienced any exceptions during testing, but I can find no other reference to the issue other than the comment left in the code. That comment pre-dates the import into GitHub, so it's rather old at this point.

My working theory is that the text not showing initially sometimes in the FormsTextBox is a timing issue of when the element is added to the visual tree when there are many layouts nested. What I believe happens with this change is the elements are added later in the process when Packager.Load is executed in the Loaded event vs just called in SetElement method of VisualElementRenderer.

Fair warning, my theory could be totally wrong too!

Other similar issues where the FormsTextBox is not as deeply nested in layouts seem to work file with the changes in #11140 (minus the Focus() call) #8503 as an example

Issues Resolved

• fixes [Bug] Entry text initially invisible on UWP #8787
• fixes [Bug] UWP ScrollView Entry's text is blank until a tap on it #11259

API Changes

None

Platforms Affected

• UWP

Behavioral/Visual Changes

Ensures Entry text is visible immediately

Before/After Screenshots

Not applicable

Testing Procedure

Issue page 8787 and full UI tests

PR Checklist

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

[bmacombe]

@PureWeen @samhouts Would you please start a build so the UI tests are run. I tried running locally and had all types of issues, I can't trust the results. Manual testing seems good.

[velocitysystems]

@bmacombe I wonder if this may also be a possible fix for #10977?

[bmacombe]

@velocitysystems It's possible, looks like there is a repro in 10977 so maybe I can build a test page and see. Or go figure out how to build the nugets locally and test against the repro project.

I've wondered in the code added back in this PR was needed late last year, but didn't need it to fix what I was after at the time and was scared of adding it back!

[bmacombe]

I see a lot of test failures, but I'm not convinced they are from the change. They remind me of a lot I get when I try running these myself.

@PureWeen Any thoughts here? Any more tips other than the instructions in the read me to run locally? I'd like to up my PR testing game a bit after this! :)

[bmacombe]

Added Issue page for #11259 to show this resolves that issue

[bmacombe]

@velocitysystems this PR won't fix #10977 😞 . I created a test page and it behaves the same.

I'm going to submit another PR with that test page and maybe look into it a bit

[PureWeen]

@bmacombe do you think this will fix #11326

[bmacombe]

@PureWeen Just tested against the repro in 11326, doesn't appear so

[bmacombe]

@PureWeen Looked at 11326 that's interesting, I confirmed that #11207 was applied to this branch also

[bmacombe]

@PureWeen Thanks, I'll pull down the updates to this branch and run them locally tonight or tomorrow.

I think that error is when it tries to navigate to a gallery page to start running tests, but it doesn't actually navigate, right?

[PureWeen]

Yea for whatever reason :-/

I think this branch was running against older navigation code. It's been a "fun" journey trying to work around all the weirdness with entering text into a SearchBox. That box is an AutoSuggestBox which the App Driver seems to have some issues entering text into.

I added code so that it should attach an attachment to the test if it fails to navigate and a bunch of stuff to help it along. I don't see the attachments with this run so I think the rebase should help

[bmacombe]

Well, it was filling in the SearchBar better this time around...but then I think it failed with the other error I see a lot.

I thought I saw some commits by you somewhere to help deal with that one too?

[PureWeen]

@bmacombe do you see that same exception when running on the 4.7.0 main branch? Usually that exception means the app crashed. Most of the code I've added is in place to recover from a crash so subsequent tests don't fail.

One thing with the UWP tests :-/
I need to get a runsettings file setup for this but on your search filter set this
-Trait:"UWPIgnore" -Trait:"CollectionView" -Trait:"Shell" -Trait:"CarouselView"

We don't run those categories on CI

The run is taking awhile though
https://dev.azure.com/xamarin/public/_releaseProgress?_a=release-environment-logs&releaseId=1984&environmentId=74746

So, I'm thinking it's still having some issues running

[bmacombe]

I'll give 4.7 a run and see what happens...if this PR is causing all those failures, will probably need to abondon it and try another approach. Though I myself am out of ideas at this point to try. Gotta be something in the Forms UWP platform though becuase I can't repro this is straight in a uwp app.

Thx for the tips on the filters. I'll also try running the test is debug and see if I can capture the any crashes.

Are all the needed scripts, yaml, etc in the repro to clone your Dev ops setup? Was thinking that could be handy to setup myself.

Thanks for all the help with this!

[bmacombe]

I didn't apply the filters, but just a run with 4.7 was similar locally

[PureWeen]

Thank you for all your help!!

I plan on documenting this in the next couple weeks but basically you can run this from and command prompt with administrator access. Administrator access is needed to install the cert to deploy the app

This first one only has to run once to install cake
dotnet tool update cake.tool -g

This will build everything and deploy

dotnet cake --target=cg-uwp-run-tests --NUNIT_TEST_WHERE="(cat != Shell && cat != CollectionView && cat != UwpIgnore && cat != CarouselView && cat != ManualReview) && ( cat == ViewBaseTests)"

If you already have everything built you can just run (notice the prefixed underscore)

dotnet cake --target=_cg-uwp-run-tests --NUNIT_TEST_WHERE="(cat != Shell && cat != CollectionView && cat != UwpIgnore && cat != CarouselView && cat != ManualReview) && ( cat == ViewBaseTests)"

I always run the full one just to be super sure all the latest bits are running :-)

I also modified the code slightly and kicked off the tests again to make sure it's taking a screen shot when it fails to navigate

[bmacombe]

@PureWeen

Thanks for the info, I'll give that all a try tomorrow.

Your welcome for the help I'm glad to pitch in some and it's been a great learning experience too!

[bmacombe]

Hey @PureWeen, I was able to run the tests with the cmd line instructions you gave me on 4.7 and they worked great all passed. When I switched back to the branch for this PR got that missing char on the first run. I did a git clean and tried it again, same result. It's possible that the change in this PR is causing that...but I'm not sold, I might swap that SearchBar for a Entry and run them again.

And thanks for those cmd line instructions...works so much better!

[bmacombe]

@PureWeen I switched to entry and I had to "help" the tests twice where even though it pulled up the right gallery page, it didn't open it.

But looks like they all passed when I did that. I wonder if the change in this PR is messing with the automation in some way?

[bmacombe]

Cleaned up and ready for a final look I hope!

[bmacombe]

I forgot about the 11259 test page I added in this PR when testing...but it's still good!