docs: modernize typography, spacing, and code block design - #6803
Conversation
mythical-fred
left a comment
There was a problem hiding this comment.
The typography/color/spacing work looks great — dark brand-tinted code surface with the custom Prism palette is a real improvement, and the table/alert/nav polish reads calmer than what's on main.
Two blockers before this lands, both about JS that got removed or referenced but never added:
-
MDXComponents.tsdeletion breaks all<LiteYouTubeEmbed>docs. The file was the sole place whereLiteYouTubeEmbedwas registered as a global MDX component (via@theme-original/MDXComponents+ spread +LiteYouTubeEmbed). Removing it leaves<LiteYouTubeEmbed id="…" />in the following files with no component to resolve to, so those pages will either fail the MDX compile or render nothing:docs.feldera.com/docs/literature/videos.mdx(five embeds)docs.feldera.com/docs/use_cases/real_time_apps/part1.mdxdocs.feldera.com/docs/use_cases/batch/intro.mddocs.feldera.com/docs/use_cases/fine_grained_authorization/intro.md
The
react-lite-youtube-embeddep is also still inpackage.json, which is a good hint that the removal was accidental. Either keep the file (its only purpose was that registration), or importLiteYouTubeEmbedexplicitly at the top of each.mdx/.mdthat uses it (note: bare.mdfiles can't import; those would need to be renamed to.mdx). -
.table-scroll-containerCSS is dead code. The new rules incustom.cssare gated on a wrapper element (.theme-doc-markdown .table-scroll-container, then.theme-doc-markdown .table-scroll-container table) that nothing in the repo produces. The inline comment says "MDXComponents wraps every markdown table in .table-scroll-container" — but (a) the pre-existingMDXComponents.tsnever did that (it only addedLiteYouTubeEmbed), and (b) this PR deletes it entirely. Result: tables on the rendered site fall through to the base.theme-doc-markdown tableblock below (which now hasborder: noneon cells and no wrapper), so tables get quiet borderless cells but lose the rounded/bordered container the design comment describes.Fix is one of: (a) actually swizzle
@theme/MDXComponents(or@theme/MDXContent) to wraptableelements in a<div className="table-scroll-container">, (b) drop the wrapper-dependent rules and move the border/radius/overflow onto.theme-doc-markdown tabledirectly, or (c) drop them entirely.
Smaller notes (non-blocking):
docusaurus.config.ts:themeanddarkThemeboth set tofelderaCodeTheme. Intentional since the code block is dark in both modes — fine, just worth a one-line comment saying so, since the two-key API otherwise looks like a mistake.- The Prism theme's rationale comment about
propertycontrast is genuinely helpful — keep that style of "why this color, verified against this background" for anyone tweaking the palette later. SandboxButton/index.tsx: removing the strayconsole.log('styles', styles)is a nice drive-by, thanks.
Once (1) and (2) are addressed I'm happy to re-review — the rest of the CSS work reads solid.
321205c to
0c7a1d7
Compare
0c7a1d7 to
e9fb93b
Compare
mythical-fred
left a comment
There was a problem hiding this comment.
Both blockers from my previous review are addressed by 1a8562ac:
MDXComponents.tsxrestores theLiteYouTubeEmbedregistration.- A
tablemapping wraps<table>in the.table-scroll-containerdiv, so the new table CSS is actually applied.
The follow-up e9fb93be (consistent SQL casing across docs) is unrelated cleanup and looks fine. Approving.
Making the docs easier on the eyes w.r.t styling and contrasts.
Describe Manual Test Plan
Lot's of visual checking.
Checklist
Breaking Changes?
Mark if you think the answer is yes for any of these components:
Describe Incompatible Changes