Fix paragraph/heading blocks always reporting full offered width - #158
Closed
Ramon Ferreira (ramonfsk) wants to merge 4 commits into
Closed
Fix paragraph/heading blocks always reporting full offered width#158Ramon Ferreira (ramonfsk) wants to merge 4 commits into
Ramon Ferreira (ramonfsk) wants to merge 4 commits into
Conversation
An unqualified Spacer() inside SingleBlockView's .paragraph/.heading HStack absorbs all leftover width from whatever the HStack is proposed, so the block always reports the full proposed width upward regardless of how little the text actually needs - defeating any ancestor that hugs its content to a capped width (e.g. a chat bubble). Render ParagraphView directly instead; left-alignment already comes from UITextView.textAlignment and BlockView's own leading VStack, not from the Spacer. Draft issue writeup in ISSUE_DRAFT-paragraph-hugging-width.md - not filed upstream yet, to be opened as an issue/PR in a future session.
BlockViewWidthHuggingTests proves the fix: a short paragraph/heading now resolves to a width close to its actual text needs when proposed more room than that (via UIHostingController/NSHostingController sizeThatFits(in:)), while a paragraph that needs to wrap still fills the proposed width - no regression to wrapping. Re-recorded three pre-existing snapshot fixtures affected by the same root cause: testCitations, testCustomBlockSpacing, and testMarkdownLists_uikit each contain a paragraph that previously wrapped one or more words earlier than the width actually available to it, because the Spacer() in the old HStack constrained what width was proposed to ParagraphView for wrapping - not just what was reported back upward. Text, formatting, and left-alignment are pixel-identical; only line-break position changed to use the full available width. macOS references for these three fixtures still need re-recording via the "Record macOS Snapshots" workflow in a follow-up commit. Refs microsoft#157 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same root cause and same fixtures as the iOS re-recording: text now wraps using the full width actually available to it instead of a narrower width caused by the old Spacer() HStack negotiation. Refs microsoft#157 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes SingleBlockView paragraph/heading layout so those blocks no longer report the full proposed width upward (previously caused by an HStack { …; Spacer() } wrapper), enabling correct width-hugging behavior and more accurate text wrapping.
Changes:
- Remove the
HStack + Spacer()wrapper around.paragraphand.heading, renderingParagraphViewdirectly. - Add a new
BlockViewWidthHuggingTestsregression test suite to validate hugging vs. wrapping behavior. - Add a root-level issue draft markdown file (likely unintended to ship with the library PR).
Reviewed changes
Copilot reviewed 3 out of 21 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
Sources/MarkdownText/UI/BlockView.swift |
Removes the HStack/Spacer wrapper for headings/paragraphs and documents the rationale. |
Tests/MarkdownTextTests/BlockViewWidthHuggingTests.swift |
Adds regression tests measuring resolved width via hosting controllers under a capped proposal. |
ISSUE_DRAFT-paragraph-hugging-width.md |
Adds a draft issue write-up to the repo root (appears unrelated to shipping code/tests). |
- remove the unused Foundation import from BlockViewWidthHuggingTests - mention NSTextView's paragraph-style alignment alongside UITextView in SingleBlockView's comment, since the claim applies to both platforms, not just iOS - remove ISSUE_DRAFT-paragraph-hugging-width.md, a local planning note that shouldn't have shipped with the library/test diff Refs microsoft#157 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Thanks for the PR! this has been fixed in #162 |
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.
Summary
fixes
SingleBlockView's.paragraph/.headingcases always reporting the full offered width upward, which breaks any ancestor that hugs its content to a capped width (the standard SwiftUI pattern for chat bubbles).SingleBlockViewwrappedParagraphViewinHStack(spacing: 0) { ParagraphView(...); Spacer() }. the trailingSpacer()has nominLength, so it absorbs all leftover width from whatever the HStack is proposed — meaning the block reports the full proposed width upward regardless of how little the text actually needs. drop the wrapper entirely; renderParagraphViewdirectly. this is safe because left-alignment already comes from the text itself (UITextView.textAlignment/NSTextViewparagraph-style alignment on macOS), not from the removedHStack, andBlockView's outerVStackis alreadyalignment: .leading.turns out the bug is broader than "hugging" alone. the same
Spacer()negotiation also constrains the width actually proposed toParagraphViewfor text wrapping — so paragraphs wrapped one or more words earlier than the space available to them, even with no capped/hugging ancestor at all. re-recording the library's own existing snapshot fixtures after the fix shows the same effect in completely ordinary, full-bleed usage: e.g. "This is a paragraph with some text to render." wraps to one line instead of two now. text, formatting, and left-alignment are unaffected — only where lines break, always using more of the available width, never less.Closes #157
Screenshots
testCitationsandtestCustomBlockSpacing, before (main) vs. after (this branch) — same text, same formatting, wrapping to fewer/wider lines:Validation
BlockViewWidthHuggingTests: a short paragraph/heading proposed 300pt resolves to well under half that (viaUIHostingController/NSHostingController.sizeThatFits(in:)), while a paragraph that needs to wrap still fills the proposed width — no regression to wrapping behaviormake test— full suite passes, including three pre-existing fixtures re-recorded for the reason above (testCitations,testCustomBlockSpacing,testMarkdownLists_uikit, iOS and macOS variants): diffed old vs. new pixel-by-pixel, confirmed text/formatting/alignment identical, only line-break position changedOSS readiness