Skip to content

content: Handle bold-text setting in code blocks and error-unimplementeds #691

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

Merged
merged 4 commits into from
May 20, 2024

Conversation

chrisbobbe
Copy link
Collaborator

@chrisbobbe chrisbobbe commented May 20, 2024

Screenshots show the app with the system bold text setting on.

Fixes: #690

Before After
728B9036-3F4C-4E00-AAA5-4ED34CDD88CE B6815335-22C0-4B2F-9A21-D2190A67018B

…teds

Code blocks and _errorUnimplemented were setting their own
weightVariableTextStyle without passing a BuildContext, which meant
the styles didn't condition on the device setting.

But weightVariableTextStyle is already applied, in a
DefaultTextStyle that wraps the whole message list. (That's because
that DefaultTextStyle wants to set a font, kDefaultFontFamily,
that's also variable-weight.) So the clobbering ones aren't
necessary; remove them.

Fixes: zulip#690
@chrisbobbe chrisbobbe added a-content Parsing and rendering Zulip HTML content, notably message contents integration review Added by maintainers when PR may be ready for integration labels May 20, 2024
@chrisbobbe chrisbobbe requested a review from gnprice May 20, 2024 03:13
@chrisbobbe chrisbobbe changed the title msglist: Handle bold-text setting in code blocks and error-unimplementeds content: Handle bold-text setting in code blocks and error-unimplementeds May 20, 2024
@chrisbobbe chrisbobbe added the a-a11y Accessibility label May 20, 2024
@gnprice
Copy link
Member

gnprice commented May 20, 2024

Thanks! Looks good; merging, with one nit:

    -    text [nfc]: Remove some params that equal defaults
    +    text [nfc]: Remove some arguments that equal defaults

because I'd read "remove some parameters" to mean changing the signature of a function, rather than some of its call sites. See:
https://developer.mozilla.org/en-US/docs/Glossary/Parameter#parameters_versus_arguments

(It's common to use the terms interchangeably and context usually makes the meaning clear, but here in the terseness of a summary line there's not much context.)

@gnprice gnprice force-pushed the pr-code-block-bold-setting branch from d72d39e to fd32618 Compare May 20, 2024 20:05
@gnprice gnprice merged commit fd32618 into zulip:main May 20, 2024
1 check passed
@chrisbobbe chrisbobbe deleted the pr-code-block-bold-setting branch May 20, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-a11y Accessibility a-content Parsing and rendering Zulip HTML content, notably message contents integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

content: Code blocks don't respond to system bold-text setting
2 participants