Support the CSS min-width property - #104
Conversation
SwiftTextCSS resolved `width` and `max-width` but had no notion of `min-width` at all, so the declaration was dropped on the floor and a box could be laid out narrower than its author-specified minimum. Add `min-width` to ComputedStyle, parse it in the resolver alongside `width`, and honour it in `copyLonghand` so `min-width: inherit` and friends work. It is deliberately absent from `inheritedProperties`: min-width is not an inherited property. In block layout the used width is now clamped up to the minimum after the max-width cap, so min-width wins when the two conflict, per CSS 2.1 §10.4. `min-width: auto` resolves to nil and floors at 0, leaving existing layouts unchanged. The CSS plumbing here was recovered from the abandoned agent/issue-97 branch, whose original issue was fixed differently by #96 and #103; that branch's competing table column-sizing changes are deliberately not carried over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae5b548861
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let minimumWidth = style.minWidth.resolved(percentageBasis: basis) ?? 0 | ||
| let cappedWidth = min(unclampedWidth, maximumWidth ?? unclampedWidth) | ||
| let contentWidth = max(0, max(cappedWidth, minimumWidth)) |
There was a problem hiding this comment.
When a table cell's min-width exceeds its intrinsic content width, tableColumnWidths still positions columns using maxContentWidth and minContentWidth, neither of which accounts for style.minWidth. This clamp therefore widens the cell only after the grid has fixed narrower column positions, causing it to overlap subsequent cells. The shipped Markdown styles already specify th, td { min-width: 4em; } in Sources/SwiftTextHTML/MarkdownToHTML.swift, so ordinary tables containing short values trigger this regression; incorporate the minimum into intrinsic column allocation before laying out the cells.
Useful? React with 👍 / 👎.
| let minimumWidth = style.minWidth.resolved(percentageBasis: basis) ?? 0 | ||
| let cappedWidth = min(unclampedWidth, maximumWidth ?? unclampedWidth) | ||
| let contentWidth = max(0, max(cappedWidth, minimumWidth)) |
There was a problem hiding this comment.
For an image whose explicit or intrinsic width is below min-width, the image branch later derives usedWidth solely from width, height, and intrinsic dimensions and overwrites the clamped box.width. Consequently, <img style="width:50px; min-width:200px"> remains 50px wide, and an auto height is not scaled with the minimum. Apply the clamp to the image's used width before deriving its auto height.
Useful? React with 👍 / 👎.
RenderPDFTests.swift was at 1196 lines before this branch; adding the min-width layout test pushed it to 1220 and tripped SwiftLint's file_length limit of 1200. Follow the precedent set by #95 (which split the text-decoration tests out for the same reason) and give used-width geometry its own suite: the existing box-model test moves across alongside the new min-width one, with the same private layoutTree/firstBlock helpers each split-out suite in this target already declares for itself. RenderPDFTests.swift is back to 1187 lines; swiftlint --strict is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Problem
SwiftTextCSSresolvedwidthandmax-widthbut had no notion ofmin-widthat all —grep -rn "minWidth\|min-width" Sources/onmainreturns nothing. The declaration was parsed as an unknown longhand and dropped, so a box could be laid out narrower than its author-specified minimum with no diagnostic.Change
ComputedStyle— newminWidth: Lengthproperty, initial value.auto.StyleResolver— parsemin-widthalongsidewidth, and handle it incopyLonghandsomin-width: inherit | initial | unsetresolve correctly. Deliberately not added toinheritedProperties:min-widthis not an inherited property.Layout— the used width is now clamped up to the minimum after the max-width cap, so min-width wins when the two conflict, per CSS 2.1 §10.4.min-width: autoresolves toniland floors at0, so existing layouts are unchanged.Tests
Two added, both verified non-vacuous (they fail with the clamp reverted):
CascadeTests.minWidth— computes px and percent, does not inherit to children, initial value isauto.RenderPDFTests.minWidthClamp— min-width raises a narrow box, outranks a conflicting max-width, and leaves a wider box alone.Full suite: 581 tests in 62 suites passing.
Provenance
The CSS plumbing was recovered from the abandoned
agent/issue-97branch (commit2437583), found during an audit for work lost to concurrent agents. That branch was a superseded alternative fix for #97, which was resolved differently by #96 and #103 — so its competing table column-sizing changes inLayout.swiftare deliberately not carried over, only the CSS property support plus a correct generic clamp.🤖 Generated with Claude Code