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

Try and better settle focus after a focused widget is removed #943

Closed
wants to merge 7 commits into from

Conversation

davep
Copy link
Contributor

@davep davep commented Oct 18, 2022

Seeks to resolve #939.

davep added 2 commits October 18, 2022 13:07
This change ensures that, for example, the widget remains in the
Screen.focus_chain so that it's easier to handle passing off focus on
removal.

To help address Textualize#939.
Rather than call on Screen._reset_focus, which seems to only really consider
siblings, instead call on the being-removed widget's screen to find the
previous widget that can receive focus.

Addresses Textualize#939.
@davep davep requested a review from willmcgugan October 18, 2022 12:23
davep added 2 commits October 18, 2022 16:01
Added to allow checking where focus can and should go without actually
causing focus to move. Initially added to aid resolve Textualize#943.

Also mote that Screen._move_focus has been rewritten to use
Screen.focus_from (really the bulk of Screen.focus_from was taken from
Screen._move_focus).
@davep davep requested a review from darrenburns October 18, 2022 15:18
remove_widgets = widget.walk_children(
Widget, with_self=True, method="depth", reverse=True
# Get the list of all the widgets that are about to be removed.
remove_widgets = list(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I've note made the suggested change to walk_children here (have it return a list) -- I didn't want to head off into a side-quest while working on this. If we're happy with this particular PR I'll then go tweak walk_children and come back and tidy this up.

@davep
Copy link
Contributor Author

davep commented Oct 19, 2022

Closed in preference to #954

@davep davep deleted the bug/939/loss-of-focus branch January 25, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant