Document image support; add Images demo and snapshot test - #146
Merged
Conversation
Finalize block-level image support (parts 1-4 already shipped) with the non-rendering deliverables: - README: move Images into the supported list with an accurate description of the experimental ImageConfig source types and fullscreen viewer. - Sample app: add an Images demonstration (enum case, subtitle, fixture) showcasing asset-catalog, bundled-resource, and remote sources, plus image-alongside-text splitting and an unresolved-source placeholder. - Tests: add ImageBlockSnapshotTests rendering a deterministic bundled image via Image(mdImage:); bundle test resources via Package.swift and record the iOS reference snapshots. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
There was a problem hiding this comment.
Pull request overview
This PR completes the non-rendering deliverables for the library’s experimental block-level Markdown image support by updating public docs, adding a dedicated sample-app demonstration/fixture, and introducing snapshot coverage backed by test-bundled resources.
Changes:
- Document images as supported in
README.md, including the experimentalImageConfigsource types and fullscreen viewer. - Add an “Images” sample-app demonstration and
images.mdfixture showcasing asset-catalog, bundled-resource, remote allowlist, and blocked-source placeholder behavior. - Add a new
ImageBlockSnapshotTestssnapshot test and bundle test resources viaPackage.swifttest-target resources.
Reviewed changes
Copilot reviewed 5 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/MarkdownTextTests/ImageBlockSnapshotTests.swift | Adds snapshot coverage for rendering a resolved, bundled test image via Image(mdImage:). |
| README.md | Moves Images into the supported feature list with a brief capability/configuration description. |
| Package.swift | Bundles test resources (Tests/MarkdownTextTests/Resources) into the test target via SPM resources. |
| Examples/.../Resources/Fixtures/images.md | Adds a dedicated images fixture covering asset-catalog, bundled-resource, remote allowlist, inline-splitting, and blocked-source placeholder. |
| Examples/.../Demonstrations.swift | Adds .images to the sample app’s demonstration list with subtitle and fixture mapping. |
Addresses review comments on the image-support PR: - Add `MarkdownListener.resolveBundledResource(fileName:ext:)` (default returns nil). The renderer now resolves bundled-resource images from the app's main bundle first and falls back to the listener when absent, so consumers can serve images that live in a dependency package or framework bundle rather than the app target. Threaded through BundledResourceImage, the fullscreen viewer, and the image-tap payload. - README: move Images up under Paragraphs in the supported list. - Tests: parse Markdown containing a bundled image and verify end-to-end resolution via the listener (the resource lives only in the test bundle); cover the no-listener case; render the image deterministically via Image(mdImage:) for the visual snapshot. Replace the helper's fatalError with throwing XCTUnwrap so a missing resource is a normal test failure. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
Addresses the second review round on PR #146: - Replace the bundled-image snapshot with the deterministic failure case: a remote image whose host is not in the allowed domains resolves to nil and renders BlockImageFailureView synchronously. The remote and bundled loading paths render asynchronously via `.task`, which the synchronous snapshot harness captures as the loading placeholder, so they can't be snapshotted deterministically. - Remove the direct Image(mdImage:) snapshot and the bundled-resource pipeline-logic tests from the snapshot file; move listener-fallback resolution coverage into ImageTapTests as a fast unit test. - Make ImageData.makeMarkdownImage(controller:) take a non-optional MarkdownController (the default only existed for test convenience); unwrap the environment controller in BlockImageView before building the payload. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
Recorded via the Record macOS Snapshots workflow (runs/29458940981) so the macOS SPM CI job has references for the new failure-placeholder snapshot. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
Addresses the latest review comments on PR #146: - Snapshot: the failure-placeholder test now parses a paragraph before and after the image, so the rendered DocumentView shows text/image/text. - Revert makeMarkdownImage(controller:) to an optional MarkdownController. SwiftUI's environment injection does not guarantee a non-nil controller, so BlockImageView optionally chains onImageTap rather than guarding out. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
Regenerated via the Record macOS Snapshots workflow (runs/29460803999) after surrounding the snapshot image with text. Refs #89 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17e2748f-66c1-4deb-b557-df65a17c84f5
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
Finalize block-level image support (the rendering work shipped in parts 1–4, #140–#143) with the remaining non-rendering deliverables: docs, a sample demo, and snapshot coverage.
ImageConfigsource types (remote-allowlist, asset-catalog, bundled-resource) and the built-in fullscreen viewer.Imagesdemonstration (Demonstration.imagescase + subtitle +images.mdfixture) showcasing asset-catalog, bundled-resource, and remote sources, image-alongside-text splitting, and an unresolved-source placeholder. Reuses existing sample assets and the allowlistedmarkdownguide.orgremote host.ImageBlockSnapshotTests, rendering a deterministic bundledsample-landscape.pngthrough the library'sImage(mdImage:)API. Bundles test resources viaresources: [.process("Resources")]inPackage.swiftand records the iOS reference snapshots.Refs #89
Validation
make test— 114 tests, 0 failures (includes the newImageBlockSnapshotTests)make lint— 0 violationsmake build-sample— BUILD SUCCEEDEDOSS readiness