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.

Sign up for GitHub

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

Jump to bottom

AUTO_FOCUS targets first focusable widget. #2581

Merged
merged 8 commits into from
May 17, 2023
Merged

AUTO_FOCUS targets first focusable widget. #2581

merged 8 commits into from
May 17, 2023

Conversation

rodrigogiraoserrao
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao commented May 16, 2023 •
edited
Loading

Fixes #2578.

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this May 16, 2023
@rodrigogiraoserrao rodrigogiraoserrao marked this pull request as draft May 16, 2023 10:32
@rodrigogiraoserrao rodrigogiraoserrao marked this pull request as ready for review May 16, 2023 10:41
willmcgugan
willmcgugan reviewed May 16, 2023
View reviewed changes
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

I'm puzzled as to why so few tests broke.

I would have expected that every snapshot test with a button or other control will now render differently.

src/textual/screen.py Outdated Show resolved Hide resolved
@willmcgugan
Copy link
Collaborator

@rodrigogiraoserrao Have you checked auto focus works on the default screen?

@rodrigogiraoserrao
Simplify auto focus code.
765c7ce 
Related comments: #2581.
@rodrigogiraoserrao
Post ScreenResume to default screen.
8399a31 
When the default screen is first created it was not getting the event ScreenResume. All other screens receive a ScreenResume when first created and _all_ screens (the default one and custom screens) receive this event when they become the active screen again, so this was kind of an edge case that needed the event to be posted by hand.
Related comments: #2581
@rodrigogiraoserrao
Fix tests.
45686c8
@rodrigogiraoserrao
Merge branch 'main' into auto-focus-improv
dea133d
@rodrigogiraoserrao
Copy link
Contributor Author

@rodrigogiraoserrao Have you checked auto focus works on the default screen?

See 8399a31.

@rodrigogiraoserrao rodrigogiraoserrao added the enhancement New feature or request label May 17, 2023
@rodrigogiraoserrao rodrigogiraoserrao merged commit 179a850 into main May 17, 2023
@rodrigogiraoserrao rodrigogiraoserrao deleted the auto-focus-improv branch May 17, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willmcgugan willmcgugan willmcgugan approved these changes

@davep davep Awaiting requested review from davep

@darrenburns darrenburns Awaiting requested review from darrenburns

Assignees

@rodrigogiraoserrao rodrigogiraoserrao

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AUTO_FOCUS has no effect in most cases
2 participants
@rodrigogiraoserrao @willmcgugan