unoplatform / uno

Open-source platform for building cross-platform native Mobile, Web, Desktop and Embedded apps quickly. Create rich, C#/XAML, single-codebase apps from any IDE. Hot Reload included! 90m+ NuGet Downloads!!
https://platform.uno
Apache License 2.0
9.05k stars 734 forks source link

Removing the focused element from the visual tree will not move focus to the "next focusable element", but to the first focusable element from root. #15942

Open ramezgerges opened 8 months ago

ramezgerges commented 8 months ago

Current behavior

This is because the element is removed from the tree before the element is Unloaded. On unloading, we call SetFocusOnNextFocusableElement, which calls GetNextTabStop. GetNextTabStop won't find any tab stops because the element is already removed from the visual tree. We will then fallback to the first focusable element in the tree, which is incorrect. image

Expected behavior

No response

How to reproduce it (as minimally and precisely as possible)

<StackPanel x:Name="sp1">
  <StackPanel x:Name="sp2">
    <StackPanel x:Name="sp3">
      <Button x:Name="btn1"></Button>
      <Button x:Name="btn2"></Button>
    </StackPanel>
    <Button x:Name="btn3"></Button>
  </StackPanel>
</StackPanel>

Focus btn1 then remove it from the tree (I did this with a KeyDown callback on btn1). You should see the next focusable element (btn2) focused, but in Uno, this will cause the first focusable element from root to be focused.

Interestingly, in this repro, WinUI does something weird. It actually focuses btn3 instead of btn2.

Workaround

No response

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

No response

Affected platforms

No response

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

Youssef1313 commented 8 months ago

SetFocusOnNextFocusableElement should NOT be called in Unloaded. It should be called on Leave which we don't yet have, but we'll have when #14936 is merged