Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @Fy1ng on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
Fy1ng
marked this pull request as ready for review
September 27, 2026 01:13
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The disabled number field exposes an editable spin-button role without disabled or read-only accessibility semantics.
Review effort: Lite
Findings: None
What changed in this PR
Adds configurable Mermaid widths, natural sizing, and alignment to Markdown Preview, with corresponding settings UI support.
Changes:
- Adds Mermaid width and alignment settings.
- Implements width precedence, sizing, scrolling, and toolbar behavior.
- Adds disabled-setting handling, search navigation, and layout tests.
- Requires an accessibility fix for the disabled number field’s spin-button semantics.
| File | Description |
|---|---|
README.md |
Adds the review marker. |
crates/settings_ui/src/settings_ui.rs |
Supports disabled fields and nested setting search. |
crates/settings_ui/src/pages/mcp_servers_page.rs |
Updates layout call sites. |
crates/settings_ui/src/pages/external_agents_page.rs |
Updates layout call sites. |
crates/settings_ui/src/page_data.rs |
Defines Mermaid settings. |
crates/settings_ui/src/components/number_field.rs |
Adds disabled number-field rendering; accessibility semantics need correction. |
crates/settings_content/src/settings_content.rs |
Adds persisted Mermaid configuration types. |
crates/markdown/src/mermaid.rs |
Implements Mermaid sizing, alignment, controls, and tests. |
crates/markdown/src/markdown.rs |
Applies top-level Mermaid layout overrides. |
crates/markdown_preview/src/markdown_preview_view.rs |
Applies preview layout settings. |
crates/markdown_preview/src/markdown_preview_settings.rs |
Maps persisted settings to renderer layout. |
assets/settings/default.json |
Documents and defaults the new settings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Mermaid blocks share the Markdown content-width constraint, so wide diagrams cannot be sized independently of the surrounding text. This adds optional width controls for top-level Mermaid blocks and configurable horizontal alignment in Markdown Preview.
limit_mermaid_width)falsemermaid_max_width)800mermaid_width_follows_diagram)falsemermaid_alignment)centerAll keys are under
markdown_preview; the custom-width JSON keys remain unchanged.aria_disabledproperty leaves existing accessibility metadata unchanged when unset.true.limit_content_width = trueandmax_width = 800, their descriptions, and their behavior remain unchanged. The original whole-document container is retained when no width override is active, including alignment-only changes. Disabling the document width limit does not impose an independent 800px Mermaid cap.leftwith both width switches disabled to retain upstream Mermaid layout. The generic Markdown renderer and nested diagrams retain their original defaults and parent constraints.Testing
Automated checks on Windows at
f2f20442814845f533a7ec1796e89e96f12de559, based on upstreambda9c0bd43a8d235d82adb01ea5bc875b861ecfc:cargo test --locked -p gpui -p markdown -p markdown_preview -p settings_ui --lib: 611 passed (347 GPUI, 169 Markdown, 32 Markdown Preview, 63 Settings UI).cargo build --locked -p zed --bin zed: passed; Windows Debug executable reports product version1.23.0+dev.f2f20442814845f533a7ec1796e89e96f12de559.target/debug/zed.exe --help: passed.git diff --check: passed.Reviewer checks:
limit_content_widthwith both Mermaid width switches off; confirm there is no independent 800px cap.The author reports manual review passed. Automated validation was performed on Windows; macOS/Linux and the complete UI checklist have not been independently verified. The accessibility checks inspect rendered AccessKit metadata; a native screen-reader session has not been run. Native capture was unavailable, so no screenshot artifacts are attached.
Self-review
This implementation and these technical notes are AI-assisted and published at the author's request. The author's confirmation, translated into English:
Original author message:
The README review marker remains because
.rulesreserves its removal for the human author.Suggested .rules additions
For
crates/markdown:Release Notes: