fix(announcements): style every markdown element a body can contain - #29
Merged
Conversation
The composer accepts markdown; this renderer styled nine element types and left the rest at browser defaults, which on a dark card means an invisible `<hr>`, a borderless GFM table, a quote indistinguishable from a paragraph and an `<h4>` smaller than the body text above it. The fenced code block was worse than unstyled. A single `code` override applied inline code's pill — background, tiny type, no wrapping — to block code as well, inside a `<pre>` with no styling of its own: a release note's own install command rendered as a pill-backed run that pushed the modal wider than the viewport instead of scrolling. Exactly the content an SDK announcement is made of. `pre` now owns the background and the horizontal scroll, and tells `code` which context it is in through a small context. react-markdown v10 dropped the `inline` prop, and the usual replacement — treating `language-*` as the marker of a block — is wrong: a fence written without a language carries no className and would fall back to inline styling. Descendant CSS cannot do it either, since the inline pill sets its background through a `style` attribute no ancestor class can override. Also styled: tables (with the same scroll containment), blockquote, hr, h4-h6, del, and images — `` is markdown, so that element already existed whether or not anyone styled it; it now stays inside the card width and loads lazily with `referrerPolicy="no-referrer"`, so a body's image cannot report the reader's current URL to whatever host it points at. No rehype-raw, no dangerouslySetInnerHTML: raw HTML in a body stays inert text, and a test in this file now says so.
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
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.
The gap
The composer accepts markdown. This renderer styled nine element types (
h1–h3,p,ul/ol/li,strong,code,a) and left everything else at browser defaults — which on a dark card means an invisible<hr>, a borderless GFM table, a blockquote indistinguishable from a paragraph, and an<h4>rendering smaller than the body text above it.The fenced code block was worse than unstyled. One
codeoverride applied inline code's pill — background,text-xs, no wrapping — to block code too, inside a<pre>with no styling of its own. A release note's own install command rendered as a pill-backed run that pushed the modal wider than the viewport instead of scrolling inside it. That is precisely the content an SDK announcement is made of:The fix
preowns the background and the horizontal scroll, and tellscodewhich context it is in through a small context.Why a context and not the two obvious shortcuts:
inlineprop, and the usual replacement — treatclassName: language-*as "block" — is wrong: a fence written without a language carries no className and falls back to inline styling. There is a test for exactly that case.pre code { … }) cannot do it either: the inline pill sets its background through astyleattribute, which no class from an ancestor can override.Also styled: tables (same scroll containment — a version-comparison table outgrows a 512px modal long before it is unreasonable to write), blockquote, hr, h4–h6, del, and images.
is markdown rather than raw HTML, so that element already existed whether or not anyone styled it; it now stays inside the card width, loads lazily, and carriesreferrerPolicy="no-referrer"so a body's image cannot report the reader's current URL to whatever host it points at.No
rehype-raw, nodangerouslySetInnerHTML— raw HTML in a body stays inert text, and this file's own test now says so rather than relying on the modal's.Tests
New
Markdown.test.tsx, 9 tests; 7 of them verified to fail againstmain(the two that passed are inline code and the raw-HTML guard, neither of which this PR changes).vitest run— 180 passed (24 files).npm run buildclean.npm run lintis still not runnable in this repo: it has neither eslint nor an eslint config installed.