-
-
Notifications
You must be signed in to change notification settings - Fork 23
fix: replace mermaid code block with custom component for build procss diagram #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes refactor diagram rendering from component-based implementations to a data-driven approach using an updated Changes
Poem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/BuildProcessFlow.astro`:
- Around line 61-69: Wrap the mermaid diagram in an accessible live region: add
role="region" and aria-live="polite" (or "assertive" if immediate announcement
is required) to the container element and provide a concise aria-label
describing the diagram (e.g., "Build process flow diagram"). Specifically update
the div that contains the <pre class="mermaid"> (in BuildProcessFlow.astro) to
include role="region", aria-live, and aria-label attributes so screen readers
announce dynamic updates and know what the visual represents.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/MermaidGraph.astro`:
- Around line 2-8: The component MermaidGraph.astro declares interface Props
with ariaLabel optional but renders an element with role="img"; update Props to
require ariaLabel (remove ? from ariaLabel) or supply a sensible default when
destructuring from Astro.props (e.g., const { graph, ariaLabel = 'Mermaid
diagram', fullWidth = true } = Astro.props) and ensure any usage of role="img"
references that required/defaulted ariaLabel so the rendered image always has an
accessible label; update both the Props definition and the destructuring in
MermaidGraph.astro (and the other occurrence at lines ~83-85) accordingly.
🧹 Nitpick comments (6)
src/components/MermaidGraph.astro (1)
81-86: Prefer text injection overset:htmlfor Mermaid source.Mermaid expects a plain-text definition;
set:htmlwill parse HTML and can introduce accidental markup or injection if the graph ever comes from non-static content. Usingset:text(or inline{graph}) keeps it as text and matches Mermaid’s input expectations.♻️ Suggested change
- set:html={graph} + set:text={graph}src/content/docs/zh/docs/plugins/social-login/apple/android.mdx (1)
30-30: Localize theariaLabelto match the page language.This is a Chinese page, but the
ariaLabelis English. Consider translating it so screen readers match the locale (and apply the same pattern across other localized pages updated in this PR).💡 Example
-<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login flow diagram on Android" fullWidth={false} /> +<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Android 上的 Apple 登录流程图" fullWidth={false} />src/content/docs/es/docs/plugins/social-login/apple/android.mdx (1)
31-31: Consider localizing theariaLabelfor Spanish users.The component usage is correct with proper accessibility attributes. However, for the Spanish locale, consider translating the
ariaLabelto Spanish for screen reader users.Suggested localized aria-label
-<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login flow diagram on Android" fullWidth={false} /> +<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Diagrama del flujo de inicio de sesión de Apple en Android" fullWidth={false} />Based on learnings,
aria-labelshould be added to interactive elements, and localizing it improves accessibility for non-English users.src/content/docs/ko/docs/plugins/social-login/apple/android.mdx (1)
30-30: Consider localizing theariaLabelfor Korean users.For better accessibility in the Korean locale, consider translating the aria-label.
Suggested localized aria-label
-<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login flow diagram on Android" fullWidth={false} /> +<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Android에서 Apple 로그인 흐름 다이어그램" fullWidth={false} />src/content/docs/ja/docs/plugins/social-login/apple/android.mdx (1)
33-33: Consider localizing theariaLabelfor Japanese users.For better accessibility in the Japanese locale, consider translating the aria-label.
Suggested localized aria-label
-<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login flow diagram on Android" fullWidth={false} /> +<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="AndroidでのAppleログインフロー図" fullWidth={false} />src/content/docs/de/docs/plugins/social-login/apple/android.mdx (1)
33-33: Consider localizing theariaLabelfor German users.For better accessibility in the German locale, consider translating the aria-label.
Suggested localized aria-label
-<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login flow diagram on Android" fullWidth={false} /> +<MermaidGraph graph={appleLoginFlowGraph} ariaLabel="Apple Login Flussdiagramm für Android" fullWidth={false} />



Changes
Fixed the mermaid code block
Motivation
Martin told me to improve the docs for Capgo Build, so I am doing that
Business impact
Low - docs
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.