Repository navigation
feat: size.markdown-fit warns when markdown text runs past its block - #46
Merged
Merged
Conversation
A showcase About tab cut its definitions off mid-section at 10 units and nothing
said so: Superset scrolls a markdown block's overflow inside the block, and macOS
draws no scrollbar there. advise now estimates each block's text height from its
markdown (headings by level, paragraphs and hard breaks, lists tight, loose and
nested, GFM tables, quotes, code, rules, <br>) and its width, and compares it with
the block.
- chartwright/design/markdown_fit.py: the estimate. Type sizes, margins, Inter's
character widths and the grid widths (1536 px at a 1600 px viewport, 1276 or
1308 px beside the filter bar) measured on 4.1.4, 5.0.0 and 6.1.0; text wrapped
word by word, margins collapsed as CSS does. A low bound (never above any
release) decides; a high bound (never below, allowing for Windows scrollbars)
sizes the fix.
- size.markdown-fit (warn/info, fixable): warns when the low bound cuts letters
off on every release and says by how much ("it needs ~29.6 units at 12/12, has
10"); an info when only the last line or the padding reaches the edge. Covers
layout rows, tabs, sub-tabs, header and footer rows. No fix for a standard's
row, a block with an image, or past 100 units. Short strips under 2 units are
the same estimate: 32 px of padding plus the line's margins.
- layout.markdown-height fixes a one-line block to the height its line takes
(1.6 to 2.4 units) instead of 2, which cut a level-1 heading's padding off.
- Design brain 11.
- tools/record_markdown_fit.py records the calibration into
tests/fixtures/markdown_fit/measurements.json (17 texts, five widths, three
releases, plus renders with Windows-style scrollbars at three heights);
tests/test_markdown_fit.py checks both bounds against every measurement;
tools/ci_live_markdown_fit.py runs in the live CI job on every release.
Dashboards that set their markdown's text sizes in CSS were estimated with Superset's own sizes, so the rule missed both ways: a long block a few lines over went unreported, a small footer that fits got a warning, and captions set smaller than Superset's text were under-estimated. - markdown_fit.py now reads the spec's dashboard.css and applies it in cascade order (!important, specificity, source order) over Superset's own styles, written as CSS per release: rules on .dashboard-markdown and its elements, with descendant, child, + and ~ combinators, :first-child and :last-child, for font-size, line-height, font-weight, letter- and word-spacing, margins, padding, borders and border-spacing (px, em, rem, %; unitless line heights). Measured on every release: every .dashboard-markdown rule beats Superset's, except an h6's font size and h4-h6 weight, which the markdown component sets in its own class; heading and body line heights are factors. - The layout is now an element tree with CSS margin collapsing (li margins, p:last-child), line boxes from their inline text (a smaller <em> in a line), inline font sizes and padding (em, code) in the widths, measured Inter widths for Latin-1 and common symbols, and wide/narrow guesses for the rest per bound. - CSS it can't read that could size markdown (another font family, attribute selectors and pseudo-elements, @media, var(), a percentage margin, the block's padding, class-less rules) is named in the finding instead of the old blanket "assumes Superset's default" caveat; a theme is still named. - The rule assumes a 1440 px window (wider windows wrap less, so the fix holds there too), estimates only the releases the spec can go to (a theme leaves out 4.1.4 and 5.0.0, so a horizontal filter bar sits on top), and lets a block run under half a px over, since scrollHeight is whole px. - The fixture now holds both windows, a styled sales-review dashboard's 14 blocks under its own CSS, 570 elements' computed styles (no CSS, every property on every element, the styled dashboard's CSS) and verify passes for corpus and styled blocks at 1440. Tests check the cascade against every computed style and both bounds against every height; the live CI check covers the styled blocks too.
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.
advisenow warns when a markdown block's text runs past its bottom edge. Superset clips the overflow inside the block, or scrolls it, with no warning. A showcase dashboard's definitions block was cut off mid-section and nothing said so.Added
size.markdown-fit(warn or info, fixable). It estimates each markdown block's rendered height from its structure and width, and gives the overflow and the height it needs: "about 33 lines cut off at the bottom edge … it needs ~29.6 units at 12/12, has 10; raise the height".--fixraises the height to it, rounded up to a fifth of a unit.layout.markdown-heightnow fixes a one-line block to its fitted height rather than a flat 2 units. The old target cut a#heading's padding, and on 4.1.4 and 5.0.0 a##heading at 2 units overflowed by 1.4 px. This also covers the short-strip case: one line of text needs 1.6 units,###1.8,##2.2 (2.0 on 6.1.0) and#2.4.Docs: blocks under 1 unit are documented as drawn at 40 px (
GRID_MIN_ROW_UNITS). The spec still accepts them.Reads the dashboard's CSS (second commit). The estimate applies the spec's own dashboard CSS for markdown, then Superset's styles per release:
.dashboard-markdown, with descendant,>,+and~combinators and:first-child,:last-childand:only-child;!important, then specificity, then source order.Blocks are laid out as elements, with CSS margin collapsing. A finding now names only the CSS it can't read (another font family, attribute selectors and pseudo-elements,
@media,var()orcalc()), rather than assuming Superset's default type. On a real dashboard whose CSS changes type, the first commit warned falsely on a one-line footer and missed a block 47 px over; both now fall inside the bounds. The rule measures in a 1440 px wide window, so a fix holds at any wider width.Calibration
DashboardBuilder.tsx:326-332at 4.1.4,:311-317at 6.1.0;Markdown.jsx:106-109, andMarkdown/Markdown.tsx:126-130at 6.1.0;util/constants.ts:42.tools/record_markdown_fit.pyrecords the fixture and reproduces it byte for byte.tools/ci_live_markdown_fit.pyis a new step in the live CI job, on every release.Tests
params_drift --alland the rule table are clean.tests/test_markdown_fit.pychecks both bounds against every measurement.Design brain 11
This collides with the integration branch's "10" from #38–#44. The numbers need reconciling at merge, as noted on those PRs.
Markdown inside sketch layouts exists only on
feat/sketch-blocks(#41). Once that merges, this rule needs to read sketch legend blocks too.