-
Notifications
You must be signed in to change notification settings - Fork 397
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
Handle changes in Textual 0.48 #543
Handle changes in Textual 0.48 #543
Conversation
Oof, there's more that needs changing here it seems. |
Previously we were declaring these styles in the Python source code, but moving them into a dedicated `.css` file allows hot reloading to work when using `textual run --dev`, and causes the rules to take effect in some situations when they otherwise would be ignored or overridden. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
d5fe572
to
799f7cc
Compare
Textual 0.48 introduces a breaking change for the `TextArea` widget: line wrapping is now enabled by default. Since we don't want line wrapping, we need to disable that. We can do so by simply assigning `False` to the `soft_wrap` reactive property. Older versions of Textual don't have that property, but they totally ignore this assignment, so assigning this doesn't affect backwards compatibility. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
The snapshot diff shows that the only difference is a slight change to the rendering of some of the characters in the capture, rather than anything our users would be able to observe. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
799f7cc
to
353a4f1
Compare
TextArea.code_editor
if available
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #543 +/- ##
==========================================
+ Coverage 92.55% 92.89% +0.34%
==========================================
Files 91 91
Lines 11304 11109 -195
Branches 1581 2022 +441
==========================================
- Hits 10462 10320 -142
+ Misses 837 789 -48
+ Partials 5 0 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
... Huh. The 3 snapshots I needed to regenerate to get them to pass locally passed on Alpine, Ubuntu 22.04, and macOS, but failed in the manylinux and musllinux containers. What the heck? |
Ah - those jobs are testing with CPython 3.7, which Textual dropped support for in 0.44... OK, I think we need to skip these 3 snapshot tests for Python 3.7, then. |
32ceb64
to
fc15cb7
Compare
The latest versions of Textual only support Python 3.8 and higher, so differences from the snapshot are expected when running Python 3.7. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
fc15cb7
to
88fbf58
Compare
Hey 👋 Textual snapshot tests will fail if there is a mismatch in the escape codes Textual writes to the terminal. However, different escape sequences can result in the same visual output. A failing snapshot test does not always mean there is a visual difference, it's more of a nudge to double check things, because different escape sequences have been produced. In the case of the TextArea, we modified how it renders, which will result in some failing tests. As for the artefacts you see in the diff, I think that's just down to how the browser is rendering the SVG. We actually overlay the two SVGs and the diff is done via CSS, so I think it's just a rendering anomaly at the browser level 😄 For the |
Fair enough, that makes sense.
Sounds plausible. I was thinking some characters might have been aligned a few pixels off in the SVG from where they were in a previous version. Either way, I agree that it doesn't seem like anything to worry about. What I'm much more curious about is why switching from
That was actually a bit uglier to do, since I don't want to drop support for Textual < 0.48 just yet: we'd need to special case on the existence or non-existence of that attribute. I started off writing
I didn't notice that there were more differences between the new defaults and the ones set by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Textual 0.48 makes several changes to the
TextArea
that we need to adapt toDEFAULT_CSS
to using a CSS file (I'm not entirely sure why, but some of our CSS rules weren't being applied, and just moving them into a CSS file was enough to fix that. I know the priority of rules inDEFAULT_CSS
is funny...)It looks like some characters are being positioned ever so slightly differently within their cell, but whatever's happening there appears to be specific to the conversion to SVG, and not something that's visible through a real terminal.
Even with the SVG, outside of the "show differences" mode I can't see the difference between the old:
and the new: