feat: align image uploads with Confluence's format#728
feat: align image uploads with Confluence's format#728birjj wants to merge 10 commits intokovetskiy:masterfrom
Conversation
2b15be9 to
1efc025
Compare
There was a problem hiding this comment.
Pull request overview
Adds opt-in image alignment behavior intended to match Confluence’s GUI uploads by calculating ac:align/ac:layout from image dimensions and emitting additional ac:image attributes (original size + custom width), with configuration via CLI flag and per-file header.
Changes:
- Introduces
--image-align(and<!-- Image-Align: ... -->) to control default alignment for sub-760px images. - Extracts image dimensions on attachment preparation and threads alignment config through
MarkConfiginto renderers. - Extends the
ac:imagetemplate and updates golden HTML output + adds unit tests for the alignment/layout calculations.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| util/flags.go | Adds --image-align CLI flag and config/env sourcing. |
| util/cli.go | Plumbs ImageAlign into MarkConfig and adds CLI/header precedence logic. |
| types/types.go | Extends MarkConfig with ImageAlign. |
| metadata/metadata.go | Adds Image-Align header parsing into metadata. |
| markdown/markdown.go | Passes MarkConfig.ImageAlign into the image renderer. |
| attachment/attachment.go | Decodes local image dimensions into Attachment.Width/Height. |
| renderer/image.go | Implements align/layout/width calculations and emits new ac:image fields. |
| renderer/fencedcodeblock.go | Applies the same image macro emission for Mermaid/D2 renders. |
| stdlib/stdlib.go | Updates ac:image template to include align/layout/original/custom width attrs. |
| renderer/image_test.go | Adds unit tests for calculateAlign, calculateLayout, calculateDisplayWidth. |
| testdata/links.html | Updates expected output for an inline attachment image macro. |
| README.md | Documents Image-Align header, --image-align, and behavior thresholds. |
Comments suppressed due to low confidence (2)
renderer/fencedcodeblock.go:146
- There are blank lines containing trailing tabs/spaces in this hunk (e.g. after
r.Attachments.Attach(attachment)). This will cause unnecessary diffs and can fail formatting checks. Runninggofmt(or removing trailing whitespace) should clean this up.
r.Attachments.Attach(attachment)
effectiveAlign := calculateAlign(r.MarkConfig.ImageAlign, attachment.Width)
effectiveLayout := calculateLayout(effectiveAlign, attachment.Width)
displayWidth := calculateDisplayWidth(attachment.Width, effectiveLayout)
renderer/image_test.go:50
- This test file contains blank lines with trailing tabs/spaces (e.g. around the section separators). Please run
gofmt(or remove trailing whitespace) to keep the diff clean and avoid formatting issues.
{"Left alignment small", "left", "500", "align-start"},
{"Center alignment small", "center", "500", "center"},
{"Right alignment small", "right", "500", "align-end"},
{"No alignment small", "", "500", ""},
// Medium images (760-1799px) use "wide" layout (must be center align)
{"Center at 760px", "center", "760", "wide"},
{"Center at 1000px", "center", "1000", "wide"},
{"Center at 1799px", "center", "1799", "wide"},
// Large images (>= 1800px) use "full-width" layout (must be center align)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
renderer/image.go
Outdated
| effectiveAlign, | ||
| effectiveLayout, | ||
| attachments[0].Width, | ||
| attachments[0].Height, | ||
| displayWidth, | ||
| attachments[0].Height, |
There was a problem hiding this comment.
The PR description says the Confluence-style attributes are only emitted when --image-align is set, but this renderer always passes OriginalWidth/OriginalHeight and a non-empty Width/Height for local images. That means the output changes even when ImageAlign is empty. If you want this to be opt-in, gate these fields (or the template attributes) behind ImageAlign != "" (or a separate boolean).
There was a problem hiding this comment.
Technically true. Personally I don't think adding an explicit gate (so we don't output ac:original-width/ac:original-height if imageAlign is empty) here is worth it. The hope is that the new behavior matches Confluence's expected markup better, so there isn't a reason to fall back to not having ac:original-width and ac:original-height set.
When imageAlign is empty the following is uploaded currently:
<ac:image ac:original-width="740" ac:original-height="192" ac:custom-width="true" ac:width="740" ac:title="..."><ri:attachment ... /></ac:image>
From what I can see that is safe, and essentially matches Confluence's expected format (minus ac:align and ac:layout, which Confluence always sets).
Feel free to disagree @mrueg :) It'd be a simple change to make
|
Thanks for the review @mrueg! You and the AI caught some good things; I've gone ahead and updated the PR to match (also see comments in the individual threads). For future changes would you prefer if I open an issue first, or are you ok with straight-to-PR contributions? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This implements image uploads that better match Confluence's default image handling. Specifically this makes it possible to center align images, even if they are wider than the page is.
Why
There is a difference in storage format between a
mark-uploaded image and one uploaded through Confluence's GUI. In practice this means thatmark-uploaded images are never center-aligned, even if they're very wide (see the screenshots at the end of this PR). The difference in storage formats looks like this:That is,
markdoesn't setac:alignorac:layout, and doesn't include the width/height information.What this PR fixes
Through manual testing I have found that the
ac:alignandac:layoutin Confluence's GUI currently works like this:ac:alignac:layoutleftalign-startcentercenterrightalign-endcenterwidecenterfull-widthThis PR essentially implements the above: image widths are parsed before upload, and the
ac:align/ac:layoutis calculated based on the above table. Theac:imagetemplate is extended to also outputac:original-width,ac:original-height,ac:custom-widthandac:width, so it aligns as closely as possible to Confluence's own uploads.This only happens if the new
--image-alignflag is set, in the interest of making this a non-breaking change. You can use the new--image-alignCLI flag to globally set whether images should be left/center/right aligned if they're <760px. The flag can also be overwritten per-file using file headers.Screenshots
--image-align="center")