Skip to content

Support new Textual footer#613

Merged
pablogsal merged 3 commits intobloomberg:mainfrom
godlygeek:support_new_textual_footer
May 29, 2024
Merged

Support new Textual footer#613
pablogsal merged 3 commits intobloomberg:mainfrom
godlygeek:support_new_textual_footer

Conversation

@godlygeek
Copy link
Contributor

Textual 0.63 introduced a new Footer widget. We need some changes to work with this:

  • Because it queries the app's bindings earlier than the old footer, we need to be careful to avoid querying some widgets before they've been mounted
  • The hack we were using to force the footer to redraw with new descriptions for keybindings no longer works with the new Footer, but I found another (nicer) one
  • We need to update our snapshot tests to account for the new footer's style

@godlygeek godlygeek requested review from pablogsal and sarahmonod May 26, 2024 21:25
@godlygeek godlygeek self-assigned this May 26, 2024
@godlygeek godlygeek force-pushed the support_new_textual_footer branch from 83488fa to 386b2de Compare May 26, 2024 21:28
@codecov-commenter
Copy link

codecov-commenter commented May 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.86%. Comparing base (41248ed) to head (e2163c3).
Report is 58 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #613      +/-   ##
==========================================
+ Coverage   92.55%   92.86%   +0.31%     
==========================================
  Files          91       92       +1     
  Lines       11304    11237      -67     
  Branches     1581     2055     +474     
==========================================
- Hits        10462    10435      -27     
+ Misses        837      802      -35     
+ Partials        5        0       -5     
Flag Coverage Δ
cpp 92.86% <100.00%> (+6.92%) ⬆️
python_and_cython 92.86% <100.00%> (-2.86%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

godlygeek added 3 commits May 26, 2024 17:55
We need to update the Textual footer whenever we change a key binding.
Textual 0.63 breaks the approach that we have been using up until now.
Switch to a new approach that works with newer versions of Textual.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
The updated Footer tries to get the active bindings when it is mounted.
Since our bindings rewriting accesses the `paused` and `disconnected`
reactives, that caused those reactives to be initialized earlier than
normal, which caused their watchers to fire earlier than normal. One of
those watchers tries to query a widget which hasn't yet been mounted,
and fails.

Work around this by setting those two reactives up to not call their
watchers when they're first initialized. We may want to apply that to
other reactives as well, but for now I'm scoping this to the smallest
possible change that fixes the issue.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
The new footer has a different style. Update our snapshot tests to
expect it.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek godlygeek force-pushed the support_new_textual_footer branch from 386b2de to e2163c3 Compare May 26, 2024 21:56
@pablogsal pablogsal merged commit 0e3673d into bloomberg:main May 29, 2024
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.

3 participants