fix(desktop): render inline images as <img> in the main block row - #331
Merged
Merged
Conversation
`MarkdownInline`'s `variant` answered two questions with one value: whether refs and tags draw TUI-style (desktop) or as pill chips (mobile), and whether an asset may take a block of its own. The desktop's main outline row wants the first and not the second, so every inline image collapsed to a chip. Mobile passes no variant at all, which is the only reason it rendered the picture. `blockAssets` carries the second question now. It defaults to what the variant used to imply, so backlinks, embedded subtrees and breadcrumbs keep their chip without passing anything. The four suggestion popups move out of `BlockRow.tsx` into `SuggestPopups.tsx` behind one shared shell. The file sat one line under the size ratchet and the four were near-identical copies with no test between them, so a fix to one (the `onMouseDown` + `preventDefault` note) had to be re-applied by hand three times. 1609 lines down to 1411, baseline tightened to match. Resolves #322 Thanks to @DYNOSuprovo
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No blocking issues remain; the documentation request is a minor nit.
Review effort: Lite
Findings: 1
What changed in this PR
Fixes desktop inline image rendering and consolidates duplicated suggestion popups.
Changes:
- Adds
blockAssetsto control asset rendering independently. - Enables desktop block-row images.
- Extracts and tests shared suggestion popups.
- Updates tests, changelog, and file-size baseline.
| File | Summary |
|---|---|
crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx |
Adds independent block-asset rendering. |
crates/outl-frontend-shared/src/markdown/MarkdownInline.test.tsx |
Tests image and styling behavior. |
crates/outl-desktop/src/components/SuggestPopups.tsx |
Provides shared popup components. |
crates/outl-desktop/src/components/SuggestPopups.test.tsx |
Tests popup behavior and rendering. |
crates/outl-desktop/src/components/BlockRow.tsx |
Enables desktop images and uses shared popups. |
crates/outl-desktop/src/components/BlockRow.test.tsx |
Adds desktop image regression tests. |
CHANGELOG.md |
Documents the fix. |
.github/file-size-baseline.txt |
Updates the reduced size baseline. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+131
to
+137
| /** Whether the context permits assets (e.g. images) to render as block | ||
| * elements (`<img>`). Defaults to `false` when `variant === "inline"` | ||
| * (compact inline contexts like breadcrumbs, previews, backlinks, embedded subtrees) | ||
| * and `true` when `variant === "pill"`. Desktop's main `BlockRow` passes | ||
| * `blockAssets={true}` (with `variant="inline"`) so images render as `<img>` | ||
| * without altering the desktop text affordances on refs, tags, and links. */ | ||
| blockAssets?: boolean; |
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
`MarkdownInline`'s `variant` answered two questions with one value: whether refs and tags draw TUI-style (desktop) or as pill chips (mobile), and whether an asset may take a block of its own. The desktop's main outline row wants the first and not the second, so every inline image collapsed to a chip. Mobile passes no variant at all, which is the only reason it rendered the picture. `blockAssets` carries the second question now. It defaults to what the variant used to imply, so backlinks, embedded subtrees and breadcrumbs keep their chip without passing anything. The four suggestion popups move out of `BlockRow.tsx` into `SuggestPopups.tsx` behind one shared shell. `BlockRow.tsx` sat exactly on its recorded ceiling, so the one line this fix adds turned the ratchet red, and the four popups were near-identical copies with no test between them: a fix to one (the `onMouseDown` + `preventDefault` note) had to be re-applied by hand three times. 1609 lines down to 1411, baseline tightened to match. Fixes #322 Reported by @jes-carr Co-authored-by: DYNOSuprovo <DYNOSuprovo@users.noreply.github.com> Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
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.
MarkdownInline'svariantanswered two questions with one value: whether refs and tags draw TUI-style (desktop) or as pill chips (mobile), and whether an asset may take a block of its own. The desktop's main outline row wants the first and not the second, so every inline image collapsed to a chip. Mobile passes no variant at all, which is the only reason it rendered the picture.blockAssetscarries the second question now. It defaults to what the variant used to imply, so backlinks, embedded subtrees and breadcrumbs keep their chip without passing anything.The four suggestion popups move out of
BlockRow.tsxintoSuggestPopups.tsxbehind one shared shell. The file sat one line under the size ratchet and the four were near-identical copies with no test between them, so a fix to one (theonMouseDown+preventDefaultnote) had to be re-applied by hand three times. 1609 lines down to 1411, baseline tightened to match.Resolves #322
Thanks to @DYNOSuprovo
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.