content: Include inline styles (italic etc.) in what we pass to WidgetSpans #1821
+87
−24
Conversation
This file contains hidden or 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
rajveermalviya
approved these changes
Aug 20, 2025
There was a problem hiding this comment.
Thanks for doing this @chrisbobbe! LGTM, couple of small comments below.
lib/widgets/katex.dart
Outdated
| // Remove link color, but | ||
| // TODO(design) do we want to do that? | ||
| color: baseColor, | ||
| // italic and strikethrough are removed below, but |
There was a problem hiding this comment.
Hmm, I did a quick test and it looks like strikethrough seems to work:
There was a problem hiding this comment.
Ah yeah that's right:
- The strikethrough is not applied to the katex in the WidgetSpan
- …but it is applied to an ancestor span, and the inline-katex implementation doesn't obscure it by setting an opaque background :) so it shows through
lib/widgets/katex.dart
Outdated
| // Just for completeness, remove "wght", but it wouldn't do anything anyway | ||
| // since KaTeX_Main is not a variable-weight font. | ||
| fontVariations: [], | ||
| // Remove link color, but |
There was a problem hiding this comment.
Looks like web client currently applies the link color over inline TeX, so we probably want to handle it similarly.
There was a problem hiding this comment.
Filed #1823 for that, so probably add a TODO pointing to that.
In the inline-math case, we're about to pass the actual ambient text style, which may have italic, strikethrough, bold, or link formatting. Consolidate the code that overrides each of those, so that it reads as intentional, and add TODOs for overrides that we might not want to keep.
…tSpans Fixes zulip#1813. Fixes zulip#1819. This is NFC for inline math, since all the inline styles are overridden there. But see the previous commit for where we consolidated those overrides, making a natural place to add more if needed in the future, with TODOs for some existing ones we might not want to keep.
chrisbobbe
force-pushed
the
pr-widget-span-not-getting-inline-styles-fix
branch
from
f24d250 to
8310d37
Compare
|
Thanks! Revision pushed, expanding the comment about strikethrough and adding a commit for #1823. Here are screenshots before and after that new commit:
|
rajveermalviya
approved these changes
Aug 21, 2025
rajveermalviya
added
integration review
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.
Fixes #1813.
Fixes #1819.
Fixes #1823.
I've left out further testing because we have other pressing priorities; please let me know if something's missing or not well-explained.