RadioSet redux
#2372
Merged
RadioSet redux
#2372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With this commit a RadioSet becomes something you can tab into and out of with just one keypress; navigation of the buttons within moves to being done with the cursor keys instead. See Textualize#2368.
This is necessary now that a focused RadioSet has acquired a border colour similar to that if a focused Input.
Initially we went with a RadioSet being a simple container of RadioButtons, with the user navigating the RadioButtons like you would any other set of widgets. This was fine but it became pretty clear pretty quickly that having to tab through a non-trivial collection of buttons in a set to get to the next widget wasn't ideal. This commit, satisfying Textualize#2368, takes over the navigation of the buttons within the container, makes the container itself a focusable widget, and sets up some new bindings to allow a more natural and efficient interaction with the set.
This keeps randomly failing in Windows in CI; multiple subsequent runs gets it going in the end, normally one further fail at a time. So let's throw a wee wait on the end and see if that helps.
davep
commented
Apr 25, 2023
| @@ -295,7 +295,7 @@ def test_demo(snap_compare): | |||
| """Test the demo app (python -m textual)""" | |||
| assert snap_compare( | |||
| Path("../../src/textual/demo.py"), | |||
| press=["down", "down", "down"], | |||
| press=["down", "down", "down", "wait:250"], | |||
There was a problem hiding this comment.
Little out-of-scope tweak to the demo snapshot; in the hope that it'll stop it being a problem when running CI tests in Windows.
Based on a sample of 1 run it seems to have done the trick!
250 worked; so let's try it lower.
Waiting 100 resulted in a fail, so let's bump back up again.
willmcgugan
approved these changes
Apr 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A revisit of
RadioSetto improve how it works in the user interface. This can be considered a breaking change in that, while the look of the user interface hasn't really changed, the way the user interacts with it, and to some degree the way the developer interacts with it, has changed.The main drivers for this change are:
RadioSetin one action -- no more navigating all of the containedRadioButtons as focusable widgets in their own right.RadioButtons are now navigated using cursor keys.This makes it far easier for a user to navigate into a
RadioSet, make any change, and then move on to the next widget on a wider form.While developers should have been treating
RadioSetas a "black box" anyway, the breaking change for them is that if they didn't treat it as such, any code doing things "under the hood" might now have unexpected results.