Draw a list item's number beside the top of a leading block - #1677
Merged
Conversation
An `<ol>`/`<ul>` item leading with a `<graph>`, `<image>`, `<video>`, `<figure>` or `<tabular>` had its number drawn after all of the item's content — at the bottom of the graph, some 250px low. A browser puts a native `::marker` on the item's first line box, and the box these components render offers none in the item's own formatting context, so the marker fell through to the block end. `Li` now publishes `firstChildListItemAlignment`, the variable a `<problem>`-style list item already reads to decide the same question, and `list.css` gives an item whose lead reports `flex-start` a zero-height line box at the top of its content for the marker to land on. Leads that offer a first line of their own keep the browser's placement, so an item leading with a `<matrixInput>` or with inline math keeps its number on that line rather than being pulled to the top. `<figure>` joins the container components that forward the item's top-margin suppression to their content, so the number and the figure start on the same row; a `<caption>` is passed over when a container looks for that content, as a `<label>` already was. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Both kinds of list item mapped a lead's `listItemInlineAlignment` to the row its number goes on with their own copy of the same two-branch test. They now go through one `listItemNumberAlignmentForLead()`, so the claim that an `<li>` and a `<problem>`-style item agree about a given lead is structural rather than a comment on two definitions. A `<spreadsheet>` lead reports `flex-start` too, and measuring it turned up a case the description missed: rather than drawing the marker after all of the item's content, the browser reserved a 17px line at the top of the item for it. The number was already in the right place and the spreadsheet started a line below it; the anchor takes that line away. Covered by a Cypress test that asserts the spreadsheet starts at the top of its item (17px without the anchor), a row in the worker's per-lead matrix, and a paragraph in the changeset. Two comments in `listItemChild.ts` still said a real `<li>`'s native marker does not ride on the lead-selection chain, which this branch makes false. They now say what each kind of list item does with the alignment the chain reports, and the `<choiceInput>` `<legend>` history they were really about is stated as such. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F9QoEYrYzcLeJKxWoeheK8
… the fix A block `<choiceInput>` reports `flex-start` too, so the set of leads that take the anchor class is not "the leads with no line box" — measured, the anchor changes neither its marker row nor its item height. Four comments said it was, and the changeset for the hidden-first-child work claimed a list item of any shape now has its number beside the top of its content, which a `<matrixInput>` lead and a `<p>`-in-a-cell `<tabular>` contradict on purpose. Say what is true in each place instead. Also notes that the naming-child exclusion is by component type, so every wrapper skips a `<caption>` rather than only `<figure>`. Swept ~60 leading shapes with the anchor on and disabled to confirm the set is complete: only the leads that report `flex-start` change at all. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `<spreadsheet>` marker test passed with the anchor removed: a spreadsheet's renderer arrives in a lazily loaded chunk, and `should` retried its way onto the empty box standing in for it — a one-line item whose marker is at its own top whatever the anchor does. Every test in the section now waits for the leading block to be on the page first, and with that gate the anchor-removed build fails ten rows rather than nine. Also from this pass: - Cover the section half of the `<figure>` forward. `Figure.js` moves a `<problem>`-style item's number too, and only the `<li>` half was asserted; removing the forward now fails four tests instead of three. - Take both marker assertions through one `measureMarkerBand()`, so the "scan came up short" failure cannot be written one way in one helper and another way in the other. - Read the anchor's computed `content` through the application's window rather than the Cypress runner's, and write the expected zero-width space as a `\u200B` escape rather than as an invisible character in the source. - Assert the outside margin on the `<sideBySide>` row as well, and say accurately which rows cannot have that assertion and why: an `<image>`, a `<video>` and a `<tabular>` carry their margin on a container div. - Stop the accompanying changeset calling a leading `<tabular>` unchanged — its number did move, and what is left over is the cell's own margin — and stop this one claiming a section has "always" top-aligned a leading figure, which is what this branch adds. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The changeset closed by saying these items now line up as `<problem>`-style items always had, which is wrong for the one lead whose section side moved here too: before this branch a `<figure>` reported no alignment of its own, so a `<problem>`/`<task>`/`<part>` leading with one put its number on a baseline the figure had no text on. Say that, and stop the entry claiming wrapping never matters — `<table suppressTableNameInTitle>` around one of these blocks is a container it does not reach. The `list.css` and `list.tsx` comments named the leads that ask for the anchor without the containers that forward one, which is most of what the new tests measure. The `<image>` row of the marker matrix sourced a deliberately missing file, so it measured a broken-image box; point it at the suite's own `Doenet_Logo_Frontpage.png`. It still fails with the anchor removed. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The same explanation — which leads offer no line box, why a `"baseline"` lead must keep the browser's placement, why a block `<choiceInput>` takes the class harmlessly — was written out five times, in `list.css`, `list.tsx`, `listItemChild.ts`, `lists.test.ts` and `list.cy.js`. Keep it in `list.css`, where the rule it explains lives, and let the others say what they do and point there. Also trims the two new test helpers' docblocks and the a11y test's preamble to their load-bearing parts. Adds the one thing that was nowhere: the fix is measured in Chrome, and why that is not a per-browser override. Rewords the changeset's `<table>` sentence, which said "one container is left out" without saying which. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F9QoEYrYzcLeJKxWoeheK8
The comment justified suppressing the delegation whenever `hideChildren` is set with "such a step shows no child at all". A held-back step shows exactly one child — its `<cascadeMessage>`, whose inverted hiding rule is what the `childrenToHide` test above exists for — so the message is drawn in an item that delegates to nobody, and keeps the top margin that puts it a line below the item's number. Tracked as Doenet#1680, with the measurement and the one-line fix. Left alone here: a different defect from Doenet#1673 in a different layout path, and the alignment a reader sees once the step is revealed is already right. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
dqnykamp
added a commit
to dqnykamp/DoenetML
that referenced
this pull request
Aug 10, 2026
… the layout A six-line comment defending a dependency that is not there costs more than the fact it records: the reasoning is now one clause of the doc comment above `firstVisibleChild`, which already had to explain that a held-back step shows its `<cascadeMessage>`. Also assert the layout the held-back item rejoins with the helper every other untitled, unboxed item in `problem.cy.js` is held to, rather than only its positional outcome; and qualify Doenet#1677's still-unreleased changeset, which says flatly that a `<cascadeMessage>` no longer takes the lead — true only of a hidden one once this entry lands beside it in the same CHANGELOG. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F9QoEYrYzcLeJKxWoeheK8
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.
Closes #1673.
The bug
A browser draws a real
<li>'s native::markeron the item's first line box. A<graph>,<image>,<video>,<figure>or<tabular>renders a box that offers none in the item's own formatting context — the media containers are flex containers, a table is a table — so the marker found nowhere to go and Blink fell through to putting it after all of the item's content. Measured rows, relative to the top of the<li>:<graph size="small"><image width="200px"><video width="320px"><figure>holding a graph<tabular>with a<p>cell<spreadsheet><p>(control)The
<spreadsheet>row is the same missing line box taking its other shape, and the reason the last column matters: rather than falling through to the end of the item, Blink reserved a 17px line at the top of the item to hold the marker. The number was already where a reader looks for it, and the spreadsheet started a line below it. Giving the marker a zero-height line box of its own takes that reserved line away, so the number and the content finally start on the same row there too.The fix
LipublishesfirstChildListItemAlignment— the same variable, read off the same per-componentlistItemInlineAlignment, that a<problem>-style list item already uses to decide exactly this question inSectioningComponent. Both go through one shared mapping,listItemNumberAlignmentForLead()inlistItemChild.ts, so the two kinds of list item agree about a given lead by construction rather than by comment. Where a section draws its own number into a grid row, a real<li>can only hand the browser a line box, whichlist.cssdoes for"flex-start"leads:The zero-width space is what makes it a line box:
content: ""produces none, and neither an absolutely positioned nor a floated pseudo-element is a place Blink will put the marker (both measured).height: 0keeps the line box out of the item's height, so it costs no space — no item in the table above grows, and the two that shrink do so because the anchor takes something away: the figure's forwarded top margin, the spreadsheet's reserved line.The second
contentgives it empty alternative text, which takes it out of the accessibility tree. Dumped from Chrome's own tree via CDP for a graph-leading item: with the anchor as shipped its subtree islistitem > ListMarker + generic, byte-identical to the same item with the anchor removed, so the marker still folds into the item's text run — the #662 shape. Drop the/ ""and an extrageneric > StaticText ""appears between the two, which is exactly the node that breaks that folding. A browser without support for the alternative-text form drops the second declaration and keeps the first, so it still gets the layout fix.<figure>joins the container components that forward the item's top-margin suppression to their content, which is what makes the number and the figure's content start on the same row (and shrinks that item by 12px rather than growing it). That forward is not the<li>'s alone: a<problem>/<task>/<part>leading with a figure now top-aligns its own number and suppresses the figure's margin too, where before it put the number on a baseline the figure had no text on. A<caption>is now passed over when a container looks for the content the number lines up with, the way a<label>already was — it is drawn below the content whatever its position among the children, so<figure><caption>…</caption><graph/></figure>lines up like the other order.The still-unreleased changeset from the preceding PR (
list-item-hidden-first-child.md) named this bug as a known limitation, so it is edited here to point at the new entry instead, keeping only the two things that really are still left over.Why gated rather than unconditional
An unconditional anchor is four lines of CSS and no worker change, and it was the first thing I built. It is wrong, visibly: it pulls the number off any first line that legitimately sits lower than a plain one. A
<matrixInput>lead loses its label's row, a<table>lead loses the row its number shared with the name, an item leading with inline math has its number ride above the text baseline, and five of the nine wrapper rows this suite already carries fail (<p>,<span>,<em>, a<p>in a<span>, an<answer>in a<p>) — counted in the verification table below.<matrixInput>is listed in the issue as affected, from its measurement (marker at rows 20–42 of a 46px item). Looking at it rather than at the number: its label sits on the matrix's last row and the marker sits with the label, which is right. It is not changed here, and the new test asserts it stays that way.Tests
list.cy.js: a matrix over eight leading markup shapes —<graph>,<image>,<video>,<tabular>with a<p>cell,<figure>,<figure>with its caption written first, a graph inside a<div>, a graph in a<sideBySide>panel — plus a composite ($g) lead, a<spreadsheet>test (whose number was already on the item's first row, so what it asserts is that the spreadsheet itself now starts there too), and a<matrixInput>test for the other direction, which also asserts that the label's row really is below the item's first one.waitForBlockLeadToRender()) before measuring anything. Not decoration: the<spreadsheet>test passed with the anchor removed until that gate went in, because a spreadsheet's renderer arrives in a lazily loaded chunk andshouldretried its way onto the empty box standing in for it — an item one line tall, whose marker is at its own top whatever the anchor does.verifyListItemMarkerOnFirstRow(), states the assertion against a plain-text sibling item's marker row rather than a tolerance, so it says what the fix claims: the number lands where a text item's number lands.verifyListItemMarkerSharesRowWith()cannot ask this — its target must be a single-line element, and the whole trouble with these leads is that the top of the item holds no line of text to name. Both now take their band through onemeasureMarkerBand(), so neither can absorb a scan that came up short.listItemParagraphRoles.cy.js: the anchor declares empty alternative text and an ordinary lead gets no anchor, with axe over the same page.lists.test.ts:firstChildListItemAlignmentper lead type (ten rows), the<figure>-forwards-past-its-caption case, and the new variable added to the four<li>rows of the existing lead-selection matrix.sectioning.test.ts: a<part>leading with a<figure>reportsflex-startand forwards past the<caption>to the graph — the section half of the sameFigurechange. Both halves fail if the forward is removed.Verified in both directions with three builds of the Cypress suite:
<matrixInput>rowThe ten are the eight markup shapes, the composite lead, and the
<spreadsheet>; the five are<p>,<span>,<em>, a<p>in a<span>, and an<answer>in a<p>. Each build was rebuilt and the whole spec re-run, so the numbers are counted failures rather than reasoning about which rows ought to move.Also green:
problem.cy.js,sectioning.cy.js,choiceinput.cy.js,accessibility/basicTests.cy.js, and thelists/sectioning/tabular/group/warningsInfosworker suites. Schema regenerated (no change — the new variable is internal).Not fixed here
<li>leading with a<matrixInput>, deliberately, as above.<p>alone in a table cell keeps its 12px top margin, so a<tabular>-leading item's cell text starts a row below its number in both kinds of list. That is a cell-spacing question, not a marker one.<li><p><graph/></p></li>: the marker is still drawn at the bottom of the graph (measured 266–288 of a 283px item), the shape An <li>'s number is drawn at the bottom of a leading <graph>, <image>, <video> or <figure> #1673's notes measured under "wrapping does not change it". A<p>reports the alignment of a paragraph whatever it holds, and it is right to: this is markup the parser already rejects — Invalid children for<p>: Found invalid children:<graph>— so the answer to it is that warning, not a rule that would have to guess when a paragraph is really a wrapper.<cascade>step's<cascadeMessage>is drawn a line below its item's number (#1680). A different layout path — a section's own grid-column number, not a native::marker— and a text lead rather than a block one:firstVisibleChildsuppresses the delegation for the whole step, on the stated grounds that such a step "shows no child at all", which is false for the one child whose hiding rule is inverted. The last commit here corrects that comment and points at the issue, which carries the measurement and the one-line fix; the fix itself is left out because this PR's review had concluded and the state a reader ends up in — the revealed step — is already right.<table suppressTableNameInTitle>holding something other than a<tabular>(marker at 266 of a 283px item for a graph inside one). A<table>reports nothing, because it renders a name of its own — Table 1 — as the item's first line, and that is the line its number belongs on; suppressing the name leaves an empty heading and no line to have it on. Giving<table>the forwarding that<figure>gets here would move the number off the name in the ordinary case, which is one of the rows the unconditional version fails. Written down in the changeset, since it is the one hole an author can still hit with valid markup.What else takes the class, and why that is safe
The condition is the lead's reported alignment, not a list of tags, so a block
<choiceInput>— on its own or inside an<answer>— takes the class too. Measured with and without it on the same page: the marker sits at rows 0–22 either way and the item's height is identical, because the label's own line box was already at the top of the item and #1668 had already stopped the<legend>from moving the marker off it.choiceinput.cy.jsand the existing wrapper matrix say so.To confirm nothing that needs the anchor is missing from that set, and nothing that would be hurt by it is in it, I measured the marker row of ~60 leading shapes with the anchor on and with it disabled on the same page — every block tag the schema offers, including
<table>,<slider>,<codeEditor>,<orbitalDiagram>,<subsetOfRealsInput>,<pegboard>,<pre>,<blockQuote>,<stack>,<aside>,<note>,<solution>,<hint>,<feedback>,<paginator>,<displayDoenetML>,<odeSystem>,<md>,<example>, a nested<ol>, and the wrapped forms. The only items that changed are the ones whose lead reportsflex-start: the six components that report it themselves (<graph>,<image>,<video>,<tabular>,<spreadsheet>,<choiceInput>) and the containers that forward one of them — a<div>, a<sideBySide>or<sbsGroup>panel, a<figure>, a<blockQuote>, a<stack>, a<pre>. (The<spreadsheet>is the one whose number did not move; its content came up to the number instead.)Measured in Chrome, which is what Cypress runs. The fix is a plain line box rather than a browser-specific override, so a browser that already places these markers well keeps doing so; noted in
list.css.🤖 Generated with Claude Code