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

[Bug] UWP Focus Regression from PR #11140 #11348

Closed
PureWeen opened this issue Jul 7, 2020 · 12 comments
Closed

[Bug] UWP Focus Regression from PR #11140 #11348

PureWeen opened this issue Jul 7, 2020 · 12 comments
Labels
4.7.0 regression on 4.7.0 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. i/regression in-progress This issue has an associated pull request that may resolve it! t/bug 🐛

Comments

@PureWeen
Copy link
Contributor

PureWeen commented Jul 7, 2020

Description

25ffd04

Is now causing the Focus View UI test on UWP to fail

Steps to Reproduce

  1. Start Control Gallery
  2. Go to Entry Gallery
  3. Select Focus View

At this point you should se

Focus? False

Or run the UI Test for EntryUITests => _Focus

Expected Behavior

When you navigate to a page the Entry shouldn't be the first thing focused by default (that's at least my understanding of this UI Test)

Actual Behavior

The Entry is the first thing that has focus

Screenshots

image

@PureWeen PureWeen added t/bug 🐛 s/unverified New report that has yet to be verified labels Jul 7, 2020
@PureWeen PureWeen added m/high impact ⬛ 4.7.0 regression on 4.7.0 labels Jul 7, 2020
@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen Thanks...I'll check it out, I thought I looked at that page in the gallery, but I must have misread the before result.

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

The Focus() call was needed to fix #8787...#2171, #8503 are fine without it. So I'll see if I can find another method to fix 8787

@PureWeen
Copy link
Contributor Author

PureWeen commented Jul 8, 2020

awesome thank you @bmacombe

@PureWeen PureWeen removed the s/unverified New report that has yet to be verified label Jul 8, 2020
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Jul 8, 2020
@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen I think I found another solution, but I'm having issues with the UI tests. I believe I had this before.

I've tried both the 1.1 and 1.2 WinAppDriver but the automation has issues bring up the various test pages. It misses the first level Any ideas?

image

I think the issue is with the Packager being called too soon in VisualElementRenderer, I investigated this when I was working on #2172. It's a pretty low-level change so I want to make sure it doesn't break anything else.

image

2172, 8503, 8787, and 8222 all check out with this change, but I'm worried about other things.

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen PR #11350 will remove that focus call and should at least unblock this issue. I'll split a fix for #8787 into another PR since it might take more review

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

#11351 is part two of the fix and should be reviewed well before merging.

@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Jul 8, 2020
@PureWeen
Copy link
Contributor Author

PureWeen commented Jul 8, 2020

@bmacombe do you think any of the changes regressed the following tests?

Issue852TestsEntriesClickable
#11352
TestShowContextMenuItemsInTheRightOrder

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen I don't think so, I'm not sure how any of the changes in 11140 would affect the grid. I ran the test locally and it passed at the 4.7 head. I also tried at the last commit before 11140 was merged and it passed.

image

I only ran on UWP though, 11140 only made changes in the UWP platform

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen 852 might be related since the Focus goes to the last Entry, which it shouldn't

Update
Tested 4.7 head and with 11350. Broken in 4.7 and ok in 11350, so yes I'd say so

I don't think the changes actually regressed the issue with 852, just broke the test.

@bmacombe
Copy link
Contributor

bmacombe commented Jul 8, 2020

@PureWeen I don't think so on 2414 TestShowContextMenuItemsInTheRightOrder.

Tested 4.7 head and 11350 both passed on UWP

@samhouts
Copy link
Member

closed by #11350

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.7.0 regression on 4.7.0 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. i/regression in-progress This issue has an associated pull request that may resolve it! t/bug 🐛
Projects
None yet
Development

No branches or pull requests

3 participants