Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safer focus lost behavior #2959

Closed
ahimberg opened this issue Aug 20, 2019 · 2 comments
Closed

Safer focus lost behavior #2959

ahimberg opened this issue Aug 20, 2019 · 2 comments
Labels
Deliverable Major item tracked for top-level planning in ADO enhancement Needs: Dev Design Needs: PM Design New feature or request Recommend: Not Planned Recommend that issue should be given Not Planned milestone.
Milestone

Comments

@ahimberg
Copy link
Member

Proposal: Safer focus lost behavior

Summary

In Xaml, when the focused control is removed from the visual tree, focus moves to the first element in the window. This differs from other platforms (I don't know how each works, but Xaml seems uniquely extreme in this) - I believe most platforms will find the nearest focusable element or at least keep focus in the same 'window' if focus was in a flyout or subwindow.

Motivation

We have had multiple bugs in our current usage of RNW where a render() update removes the focused element. We've had to add code outside of RNW to notice when this is happening and restore focus back into the RNW control.

vnext's 'safe harbor' feature which when calling blur() will move focus to the root of the reactcontrol is perhaps part of a solution - if it also worked to safe harbor within the nearest 'window' and not just the root. Requiring the call to blur() is also a problem though if its not needed on other platforms. react-native developers on other platforms are unlikely to be aware of this Xaml behavior and some kind of 'auto-blur' behavior on removal seems desirable.

Basic example

Have a RN layout where a Touchable's onPress will re-render and create a different tree with different buttons. Focus will jump to the root of the window. If doing this in a flyout, focus jumps all the way to parent window root.

Open Questions

I attempted to start implementing this just using the current safe harbor, by checking if the current view being removed is the focused element.

  1. That wouldn't solve the sub-window case
  2. When re-rendering a tree of elements, the elements much higher in the tree are going to be removed first. So instead of checking if the view being removed is focused, I had to check if the focused element is a child of the view being removed. I wasn't sure of an efficient check for this and ended up having to walk all the parents up to the root from the focused element seeing it one of them was the element being removed.
  3. On RS4+ Platforms OnLosingFocus() could maybe be used to help with this. Thats what we did with our LivePersonaCard feature, but it didn't work on RS3 due to unavailable APIs. The app there notices focus movement and closes flyouts automatically on focus lost - fixing focus quickly isn't enough since nothing is async in the flyout manager.
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Aug 20, 2019
@chrisglein chrisglein added Needs: Dev Design Needs: PM Design New feature or request and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Aug 26, 2019
@chrisglein
Copy link
Member

@harinikmsft @licanhua This looks like it needs some design to it. Can you take a look and come up with a proposal?

@chrisglein chrisglein added this to the vNext Milestone 4 milestone Aug 29, 2019
@chrisglein chrisglein changed the title [PROPOSAL]: Safer focus lost behavior Safer focus lost behavior Aug 29, 2019
@chrisglein chrisglein added the Deliverable Major item tracked for top-level planning in ADO label Nov 26, 2019
@chrisglein chrisglein removed the vnext label Mar 18, 2020
@chrisglein chrisglein modified the milestones: 0.62 (M5), Backlog Apr 10, 2020
@harinikmsft
Copy link
Contributor

#5706 is linked to this, if we add a more sophisticated focus management system in RN, we may be able to address issues like this a bit more intuitively.

@chrisglein chrisglein assigned stmoy and unassigned harinikmsft Jan 24, 2022
@chrisglein chrisglein added the Recommend: Not Planned Recommend that issue should be given Not Planned milestone. label Aug 16, 2023
@chrisglein chrisglein closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deliverable Major item tracked for top-level planning in ADO enhancement Needs: Dev Design Needs: PM Design New feature or request Recommend: Not Planned Recommend that issue should be given Not Planned milestone.
Projects
None yet
Development

No branches or pull requests

5 participants