Tab focus #458
Tab focus #458
Conversation
willmcgugan
commented
May 3, 2022
•
willmcgugan
commented
- Tab shows focused widget, tab again moves to next widget, shift+tab moves to previous widget
- Drops css parameter on App, in favor of CSS class attribute
- Adds Tint renderable and CSS declaration
- Fixes issue where universal selector didn't respect pseudo selectors
| @@ -91,7 +91,7 @@ class BasicApp(App): | |||
|
|
|||
| def on_load(self): | |||
| """Bind keys here.""" | |||
| self.bind("tab", "toggle_class('#sidebar', '-active')") | |||
| self.bind("s", "toggle_class('#sidebar', '-active')") | |||
There was a problem hiding this comment.
The app now uses tab. Which means we can't use tab elsewhere. Is this a reasonable sacrifice?
There was a problem hiding this comment.
The only use case of letting be used in the userland space of Textual would be for widgets that let people edit some code? In which case, the equivalent of a browser's textarea could take priority over the behaviour of "pressing tab" - but I reckon this would break accessibility.
I don't know what the best practices are in terms of managing a11y and code editing?
Codecov Report
@@ Coverage Diff @@
## css #458 +/- ##
===========================================
+ Coverage 38.06% 53.37% +15.31%
===========================================
Files 85 102 +17
Lines 6526 8436 +1910
===========================================
+ Hits 2484 4503 +2019
+ Misses 4042 3933 -109
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -91,7 +91,7 @@ class BasicApp(App): | |||
|
|
|||
| def on_load(self): | |||
| """Bind keys here.""" | |||
| self.bind("tab", "toggle_class('#sidebar', '-active')") | |||
| self.bind("s", "toggle_class('#sidebar', '-active')") | |||
There was a problem hiding this comment.
The only use case of letting be used in the userland space of Textual would be for widgets that let people edit some code? In which case, the equivalent of a browser's textarea could take priority over the behaviour of "pressing tab" - but I reckon this would break accessibility.
I don't know what the best practices are in terms of managing a11y and code editing?
| @@ -87,7 +98,9 @@ class ActionError(Exception): | |||
| class App(Generic[ReturnType], DOMNode): | |||
| """The base class for Textual Applications""" | |||
|
|
|||
| css = "" | |||
| CSS = """ | |||
There was a problem hiding this comment.
Just to be sure I understand correctly... The only reason we can move this to a class-level variable that can no longer be overridden in the constructor is because we expect to never have more than one instance of App instantiated in a Python program, right? 🙂
(so customising this CSS at the class level can only be be done before instantiating an App for the first time)
There was a problem hiding this comment.
You can still define a subclass with different CSS.
I wanted to remove it from the constructor so that there is only one place to included the CSS if its a string literal. A classvar constant seems like the best place.
| @@ -233,7 +240,7 @@ async def _process_messages(self) -> None: | |||
| except CancelledError: | |||
| raise | |||
| except Exception as error: | |||
| self.app.panic(error) | |||
| self.app.on_exception(error) | |||
There was a problem hiding this comment.
@darrenburns pretty sure this is the cause of the "can't render blah..." error you saw.
|
Only other thing I want to mention here is that we might need to revisit the accessibility of this (either by default or via config) since it's just overlaying a colour. We may need an option to display some kind of marker too. |
|
Yeah, we need something other than commit to show focus. Underline in addition to colour makes sense, but we have that issue that it underlines everything. |
