You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "minyeamer (via GitHub)" <gi...@apache.org> on 2026/05/12 16:42:14 UTC
[PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
minyeamer opened a new pull request, #66809:
URL: https://github.com/apache/airflow/pull/66809
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
This PR improves how `doc_md` content renders in the Dag Documentation dialog in the new UI.
It adds proper rendering for inline code, fenced code blocks, KaTeX math, and Mermaid diagrams.
## What changed
- render inline code separately so it stays in normal sentence flow
- render fenced code blocks in a dedicated full-width container with broader syntax highlighting, line numbers, and a copy action
- add KaTeX support for math expressions and load the bundled stylesheet needed to display them
- add Mermaid support for diagram blocks with strict security settings
- add regression tests for inline code, fenced code blocks, math, and Mermaid rendering
## Test plan
- install dependencies with the committed lockfile to confirm the markdown-related packages and KaTeX assets install correctly:
`cd airflow-core/src/airflow/ui && corepack pnpm install --frozen-lockfile`
- run the focused React markdown tests to verify inline code, fenced code blocks, math, and Mermaid rendering:
`cd airflow-core/src/airflow/ui && corepack pnpm vitest run src/components/ReactMarkdown.test.tsx`
- run eslint on the touched UI files to catch typing, import, and lint regressions in the markdown rendering changes:
`cd airflow-core/src/airflow/ui && corepack pnpm exec eslint src/components/ReactMarkdown.tsx src/components/ReactMarkdownBlocks.tsx src/utils/renderMermaid.ts src/utils/syntaxHighlighter.ts src/components/ReactMarkdown.test.tsx src/main.tsx`
- build the UI bundle to confirm the new markdown packages and KaTeX styles work in a production build:
`cd airflow-core/src/airflow/ui && corepack pnpm build`
## Screenshots
| Mode | Before | After |
| --- | --- | --- |
| Light |  |  |
| Dark |  |  |
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes
Generated-by: GitHub Copilot (GPT-5.4) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239155636
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
Review Comment:
Disabling `singleDollarTextMath` should significantly reduce the risk of unintended math rendering in shared contexts such as dashboard alerts and form descriptions.
That said, I think the cleanest solution would be to scope the math plugins exclusively to the DAG/Task documentation component. I will look into adding a prop to the shared `ReactMarkdown` component to enable math only where it is explicitly needed, and apply it accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4460968217
@minyeamer — Your unresolved review thread(s) from @bbovenzi appear to have been addressed (post-review commits and/or in-thread replies on every thread, with the latest commit pushed after the most recent thread). I've added the `ready for maintainer review` label so the PR re-enters the maintainer review queue.
@bbovenzi — could you take another look when you have a chance? If you agree the feedback was addressed, please mark the threads as resolved so the queue signal stays accurate. If a thread still needs work, please reply in-line — @minyeamer will follow up.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @choo121600 before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3243007694
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
Fixed the requested changes in 2b0d38e8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4682427365
@bbovenzi Thanks for the follow-up review. I addressed both of the remaining points and pushed follow-up commits:
- Removed the 3rd-party license additions in 44c6acf7
- Changed Mermaid from the context provider back to a small utility in 39dc0f1a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238874610
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
I think it might be better to revert this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4545703644
@bbovenzi I went through your requested changes and pushed follow-up commits to address them.
The biggest change is that Mermaid now lives in a `MermaidProvider` under `src/context`, instead of being initialized from a standalone utility. That keeps theme initialization inside the React lifecycle while still lazy-loading Mermaid only when needed.
Along the way, I also incorporated related feedback from the other threads:
- reverted the syntax highlighter back to `PrismLight`
- reduced the tests to minimal smoke coverage
- limited math support to display math (`$$...$$`) and lazy-loaded KaTeX styles
- added the required license notice updates
I'm sorry for the late mention. After updating this branch from `main`, a few PROD image tests started failing, and I wanted to sort that out before asking for another look.
Also, thanks to @potiuk for pointing out that I should mention the reviewer once the requested changes were addressed.
I’ve resolved the threads I addressed. When you have a moment, could you please take another look and let me know if anything still needs adjustment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4445749662
@Srabasti @choo121600
Thanks for pointing those out! I’ve resolved the failures locally and pushed the fixes:
**Static Checks**: Fixed the pre-commit and CI image check failures.
**Provider Distributions**: All failed provider tests (Compat 3.0.6) are now passing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3349879032
##########
airflow-core/3rd-party-licenses/LICENSE-mermaid.txt:
##########
Review Comment:
Thanks, I added these based on the earlier [feedback](https://github.com/apache/airflow/pull/66809#pullrequestreview-4287101297). I’ll wait for confirmation, and if they’re not needed for the new UI, I’ll remove them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4762558981
@jscheffl Thanks, updated this in the docs and examples in #0f5935d4.
I added a short note to `airflow-core/docs/core-concepts/dags.rst` describing the Markdown features now rendered in Dag documentation, including fenced code blocks, Mermaid fences, and KaTeX display math.
For `airflow-core/docs/tutorial/fundamentals.rst`, I updated the existing section that already references `airflow-core/src/airflow/example_dags/tutorial.py`. Instead of adding a separate tutorial example, I extended the existing `print_date` task's `doc_md` so the rendered Task documentation now demonstrates inline code, a fenced code block, KaTeX, and Mermaid while still describing the tutorial task and its downstream `sleep` / `templated` tasks.
I also added new screenshots as `task_doc_md.png` and `dag_doc_md.png` for both light and dark themes. I kept the existing `task_doc.png` and `dag_doc.png` files untouched, since they may still be referenced elsewhere.
For the Dag-level `doc_md`, I added `example_markdown_doc.py` as a small example Dag whose `doc_md` demonstrates the same richer Markdown rendering features at the Dag documentation level; this file can be included in the example docs as well if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3249091804
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
Review Comment:
Applied.
1. I pulled the conversion into a shared helper, but kept it as a narrow text extractor rather than a generic `toString()`. This path receives `ReactNode`, and blind stringification could turn non-text nodes into incorrect output.
2. I dropped number handling here. This path only needs markdown code text, so keeping it string-only makes the contract tighter without losing any expected behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4833230586
@pierrejeambrun Thanks for the review — I pushed 359cbce8 and fcda8a71.
In 359cbce8, I moved Mermaid branching and KaTeX math rendering so both are handled in `ReactMarkdownBlocks`, and also updated the related docs/tutorial text to match the fenced `math` behavior.
In fcda8a71, I updated the docs images to reflect that same rendering change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239156697
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
I will replace the static `import` with a dynamic one to avoid loading KaTeX CSS as part of the app-wide initial bundle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3242954668
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -60,8 +64,8 @@ const LinkComponent = ({
href,
title,
}: {
- readonly children: ReactNode;
- readonly href: string;
+ readonly children?: ReactNode;
+ readonly href?: string;
Review Comment:
Thank you for pointing this out.
I’ve addressed the issue in 92eb8e10 by restoring the required props to match the original implementation.
Regarding the remaining requested changes, I will review and address them within the next 24 hours as soon as I have a moment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3349235909
##########
airflow-core/src/airflow/ui/package.json:
##########
@@ -119,7 +123,6 @@
"web-worker": "^1.5.0"
},
"pnpm": {
- "minimumReleaseAge": 5760,
Review Comment:
Why?
##########
airflow-core/3rd-party-licenses/LICENSE-mermaid.txt:
##########
Review Comment:
This 3rd party licenses thing, I believe are artifacts from Airflow 2.x UI. (Now I don't think that's needed).
@potiuk for confirmation.
##########
airflow-core/src/airflow/ui/src/context/mermaid/MermaidProvider.tsx:
##########
@@ -0,0 +1,55 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { PropsWithChildren } from "react";
+
+import { useColorMode } from "src/context/colorMode";
+
+import { MermaidContext, type MermaidRenderParams, type MermaidTheme } from "./Context";
+
+type MermaidApi = {
+ initialize: (config: { securityLevel: "strict"; startOnLoad: false; theme: MermaidTheme }) => void;
+ render: (diagramId: string, chart: string) => Promise<{ svg: string }>;
+};
+
+let mermaidModulePromise: Promise<MermaidApi> | undefined;
+
+const getMermaid = async (): Promise<MermaidApi> => {
+ mermaidModulePromise ??= import("mermaid").then(({ default: mermaid }) => mermaid as MermaidApi);
+
+ return mermaidModulePromise;
+};
+
+export const MermaidProvider = ({ children }: PropsWithChildren) => {
+ const { colorMode } = useColorMode();
+ const theme: MermaidTheme = colorMode === "dark" ? "dark" : "default";
+
+ const renderDiagram = async ({ chart, diagramId }: MermaidRenderParams): Promise<string> => {
+ const mermaid = await getMermaid();
+
+ mermaid.initialize({ securityLevel: "strict", startOnLoad: false, theme });
Review Comment:
That's done on every 'renderDiagram' ? Maybe just initialize once, and detect the theme change.
##########
airflow-core/LICENSE:
##########
@@ -239,6 +239,10 @@ The text of each license is also included at 3rd-party-licenses/LICENSE-[project
(MIT License) Monaco Editor v0.52.2 (https://github.com/microsoft/monaco-editor)
(MIT License) eonasdan-bootstrap-datetimepicker v4.17.49 (https://github.com/eonasdan/bootstrap-datetimepicker/)
(MIT License) Chakra UI v3.35.0 (https://github.com/chakra-ui/chakra-ui)
+ (MIT License) KaTeX v0.16.45 (https://katex.org)
+ (MIT License) mermaid v11.14.0 (https://github.com/mermaid-js/mermaid)
+ (MIT License) rehype-katex v7.0.1 (https://github.com/remarkjs/remark-math/tree/main/packages/rehype-katex)
+ (MIT License) remark-math v6.0.0 (https://github.com/remarkjs/remark-math/tree/main/packages/remark-math)
Review Comment:
Same here, not needed I think.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +134,103 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type MarkdownCodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+const hasDisplayMath = (children: Options["children"]): boolean =>
+ typeof children === "string" && children.includes("$$");
+
+const InlineCodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme) =>
+ ({ children }: { readonly children?: ReactNode }) => {
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<MarkdownCodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
- const match = /language-(?<lang>\w+)/u.exec(className ?? "");
+ const { children: codeChildren, className } = codeElement.props;
+ const match = /language-(?<lang>[-\w]+)/u.exec(className ?? "");
const language = match?.groups?.lang;
- // Safely extract string content from children
- let childString = "";
+ const codeText = Array.isArray(codeChildren)
+ ? codeChildren.map((child) => (typeof child === "string" ? child : "")).join("")
+ : typeof codeChildren === "string"
+ ? codeChildren
+ : "";
- if (typeof children === "string") {
- childString = children;
- } else if (Array.isArray(children)) {
- childString = children.filter((child) => typeof child === "string").join("");
+ const childString = codeText.replace(/\n$/u, "");
+
+ if (language === "mermaid") {
+ return <MarkdownMermaid chart={childString} fallbackStyle={style} />;
}
- return (
- <SyntaxHighlighter language={language ?? "text"} PreTag="div" style={style} wrapLongLines>
- {childString.replace(/\n$/u, "")}
- </SyntaxHighlighter>
- );
+ return <MarkdownCodeBlock language={language} style={style} value={childString} />;
};
-const ReactMarkdown = (props: Options) => {
+const createMarkdownComponents = (style: SyntaxTheme): Components => ({
+ // eslint-disable-next-line id-length
+ a: LinkComponent,
+ blockquote: BlockquoteComponent,
+ code: InlineCodeComponent,
+ del: DelComponent,
+ em: EmComponent,
+ h1: makeHeading("h1"),
+ h2: makeHeading("h2"),
+ h3: makeHeading("h3"),
+ h4: makeHeading("h4"),
+ h5: makeHeading("h5"),
+ h6: makeHeading("h6"),
+ hr: HrComponent,
+ img: ImgComponent,
+ li: LiComponent,
+ ol: OlComponent,
+ // eslint-disable-next-line id-length
+ p: PComponent,
+ pre: createPreComponent(style),
+ table: TableComponent,
+ tbody: Table.Body,
+ td: Table.Cell,
+ text: TextComponent,
+ th: Table.ColumnHeader,
+ thead: Table.Header,
+ tr: Table.Row,
+ ul: UlComponent,
+});
+
+const ReactMarkdown = ({ children, ...props }: Options) => {
const { colorMode } = useColorMode();
const style = colorMode === "dark" ? oneDark : oneLight;
+ const components = createMarkdownComponents(style);
+ const shouldEnableMath = hasDisplayMath(children);
- const components = {
- // eslint-disable-next-line id-length
- a: LinkComponent,
- blockquote: BlockquoteComponent,
- code: createCodeComponent(style),
- del: DelComponent,
- em: EmComponent,
- h1: makeHeading("h1"),
- h2: makeHeading("h2"),
- h3: makeHeading("h3"),
- h4: makeHeading("h4"),
- h5: makeHeading("h5"),
- h6: makeHeading("h6"),
- hr: HrComponent,
- img: ImgComponent,
- li: LiComponent,
- ol: OlComponent,
- // eslint-disable-next-line id-length
- p: PComponent,
- pre: PreComponent,
- table: TableComponent,
- tbody: Table.Body,
- td: Table.Cell,
- text: TextComponent,
- th: Table.ColumnHeader,
- thead: Table.Header,
- tr: Table.Row,
- ul: UlComponent,
- };
-
- return <ReactMD components={components as Components} {...props} remarkPlugins={[remarkGfm]} skipHtml />;
+ useEffect(() => {
+ if (shouldEnableMath) {
+ void katexStyleLoader.load();
+ }
+ }, [shouldEnableMath]);
+
+ return (
+ <Box alignSelf="stretch" css={markdownContentStyles} maxWidth="100%" minWidth={0} width="100%">
+ <ReactMD
+ {...props}
+ components={components}
Review Comment:
The spread order changed here so components always wins over a caller-supplied components prop (it used to be overridable). I checked the consumers and none pass components, so this looks harmless — is that intentional? (otherwise restore it)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4440500072
@minyeamer A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
- :x: **Pre-commit / static checks** — Failing: `CI image checks / Static checks`. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
- :x: **Provider tests** — Failing: `provider distributions tests / Compat 3.0.6:P3.10:`. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/12_provider_distributions.rst).
No rush.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "parkhojeong (via GitHub)" <gi...@apache.org>.
parkhojeong commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238554870
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
Thanks for the contribution.
This looks useful, but the PR currently covers three different areas: code blocks, Mermaid, and math support.
Would you mind splitting this into separate PRs, one per feature? A smaller, focused PR will be easier to review and discuss.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238722887
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
Could you share the reasoning behind switching from PrismLight with selective language registration to the default react-syntax-highlighter export here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238730527
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
The scope seems small enough that it should be fine to keep this in a single PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238874610
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
I think it might be better to revert this change 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3242368261
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
Review Comment:
Could we not just import CodeProps from chakra-ui?
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
+ }
+
+ return "";
+};
+
+const CodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
Review Comment:
```suggestion
const InlineCodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
```
Since we don't use this for multiline code blocks.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
Review Comment:
We're checking the children types twice. Let's simplify this function with a shared `toString()` converter
##########
airflow-core/src/airflow/ui/src/utils/renderMermaid.ts:
##########
Review Comment:
We should make this into a react context provider and move it to `src/context`. Then we only need to initialize mermaid theme once for anywhere we use it, theme updates exist in the react lifecycle and we can manage the `initializedTheme` as a real state instead of a mutable variable.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdownBlocks.tsx:
##########
@@ -0,0 +1,223 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Flex, Spinner, Text } from "@chakra-ui/react";
+import { useEffect, useId, useState, type ReactNode } from "react";
+import { useTranslation } from "react-i18next";
+
+import { LazyClipboard } from "src/components/ui";
+import { renderMermaidDiagram } from "src/utils/renderMermaid";
+import { resolveSyntaxLanguage, SyntaxHighlighter, type SyntaxTheme } from "src/utils/syntaxHighlighter";
+
+const MarkdownBlockFrame = ({
+ action,
+ children,
+ label,
+}: {
+ readonly action: ReactNode;
+ readonly children: ReactNode;
+ readonly label: string;
+}) => (
+ <Box maxWidth="100%" minWidth={0} my={3} width="100%">
+ <Flex
+ alignItems="center"
+ bg="bg.muted"
+ borderColor="border.emphasized"
+ borderTopRadius="md"
+ borderWidth="1px"
+ justifyContent="space-between"
+ minH={8}
+ px={3}
+ py={1}
+ >
+ <Text color="fg.muted" fontFamily="mono" fontSize="xs" lineHeight="short">
+ {label}
+ </Text>
+ {action}
+ </Flex>
+ <Box
+ borderBottomRadius="md"
+ borderColor="border.emphasized"
+ borderTopWidth={0}
+ borderWidth="1px"
+ maxWidth="100%"
+ minWidth={0}
+ overflow="hidden"
+ >
+ {children}
+ </Box>
+ </Box>
+);
+
+export const MarkdownCodeBlock = ({
+ language,
+ style,
+ value,
+}: {
+ readonly language?: string;
+ readonly style: SyntaxTheme;
+ readonly value: string;
+}) => {
+ const { t: translate } = useTranslation("components");
+ const codeBlockStyle = style['pre[class*="language-"]'];
+ const languageLabel = language ?? "text";
+ const syntaxLanguage = resolveSyntaxLanguage(language);
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label={translate("clipboard.copy")}
+ data-testid="markdown-copy-button"
+ getValue={() => value}
+ title={translate("clipboard.copy")}
+ />
+ }
+ label={languageLabel}
+ >
+ <Box
+ css={{ ...codeBlockStyle, borderRadius: 0, margin: 0 }}
+ data-testid="markdown-code-scroll-area"
+ maxWidth="100%"
+ minWidth={0}
+ overflowX="auto"
+ overflowY="hidden"
+ width="100%"
+ >
+ <Box data-testid="markdown-code-content" display="inline-block" minWidth="100%">
+ <SyntaxHighlighter
+ codeTagProps={{
+ style: {
+ background: "transparent",
+ overflowWrap: "normal",
+ whiteSpace: "pre",
+ wordBreak: "normal",
+ },
+ }}
+ customStyle={{
+ background: "transparent",
+ borderRadius: 0,
+ margin: 0,
+ padding: 0,
+ width: "max-content",
+ }}
+ language={syntaxLanguage}
+ lineNumberStyle={{ minWidth: "2.5em", opacity: 0.6, paddingRight: "1em" }}
+ PreTag="div"
+ showLineNumbers
+ style={style}
+ >
+ {value}
+ </SyntaxHighlighter>
+ </Box>
+ </Box>
+ </MarkdownBlockFrame>
+ );
+};
+
+export const MarkdownMermaid = ({
+ chart,
+ fallbackStyle,
+ theme,
+}: {
+ readonly chart: string;
+ readonly fallbackStyle: SyntaxTheme;
+ readonly theme: "dark" | "default";
+}) => {
+ const { t: translate } = useTranslation("components");
+ const diagramId = useId().replaceAll(":", "");
+ const [error, setError] = useState(false);
+ const [svg, setSvg] = useState<string>();
+ const mermaidLabel = "mermaid";
+
+ useEffect(() => {
+ let cancelled = false;
+
+ const renderDiagram = async () => {
+ try {
+ const renderedSvg = await renderMermaidDiagram({
+ chart,
+ diagramId: `markdown-mermaid-${diagramId}`,
+ theme,
+ });
+
+ if (!cancelled) {
+ setSvg(renderedSvg);
+ setError(false);
+ }
+ } catch {
+ if (!cancelled) {
+ setError(true);
+ setSvg(undefined);
+ }
+ }
+ };
+
+ void renderDiagram();
+
+ return () => {
+ cancelled = true;
+ };
+ }, [chart, diagramId, theme]);
+
+ if (error) {
+ return <MarkdownCodeBlock language="mermaid" style={fallbackStyle} value={chart} />;
+ }
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label={translate("clipboard.copy")}
+ data-testid="markdown-mermaid-copy-button"
+ getValue={() => chart}
+ title={translate("clipboard.copy")}
+ />
+ }
+ label={mermaidLabel}
Review Comment:
```suggestion
label="mermaid"
```
No need to make a variable that we only use once.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
Review Comment:
Do we even need to parse numbers? React will just treat them as strings anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3242201254
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
Review Comment:
Yes, please review your AI code first before asking others to review your PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238665174
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
Thanks for the suggestion. I agree with splitting the features.
Since I'm new to this contribution process,
I'll consult the others on whether to close this PR and open three separate ones.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "parkhojeong (via GitHub)" <gi...@apache.org>.
parkhojeong commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239030555
##########
airflow-core/src/airflow/ui/src/main.tsx:
##########
@@ -20,6 +20,7 @@ import * as ChakraUI from "@chakra-ui/react";
import * as EmotionReact from "@emotion/react";
import { QueryClientProvider } from "@tanstack/react-query";
import axios, { type AxiosError } from "axios";
+import "katex/dist/katex.min.css";
Review Comment:
Could we avoid loading KaTeX CSS globally? This makes KaTeX assets part of the app-wide cost, even on screens that never render math.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239020520
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
Review Comment:
Regarding the tests, it was automatically generated alongside the code via AI and weren't part of an intentional test plan.
I agree with your point. I'll simplify it to a minimal test.
I plan to push a new commit in a while, which will include these test refinements and the revert to `PrismLight` as discussed above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3242243302
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -60,8 +64,8 @@ const LinkComponent = ({
href,
title,
}: {
- readonly children: ReactNode;
- readonly href: string;
+ readonly children?: ReactNode;
+ readonly href?: string;
Review Comment:
These props shouldn't change. If you are using a Link without href or children, then its not a Link.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239156697
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
All newly bundled UI dependencies (`remark-math`, `rehype-katex`, `katex`, ...) are MIT-licensed. I will add the corresponding license texts to `airflow-core/3rd-party-licenses/` following the existing convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3349873885
##########
airflow-core/src/airflow/ui/package.json:
##########
@@ -119,7 +123,6 @@
"web-worker": "^1.5.0"
},
"pnpm": {
- "minimumReleaseAge": 5760,
Review Comment:
While fixing the test failure, I removed it because VS Code showed this warning: "Property minimumReleaseAge is not allowed." It looks like this setting is needed here, so I'll restore it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3306136694
##########
airflow-core/src/airflow/ui/src/context/mermaid/MermaidProvider.tsx:
##########
@@ -0,0 +1,74 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { PropsWithChildren } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+
+import { useColorMode } from "src/context/colorMode";
+
+import { MermaidContext, type MermaidRenderParams, type MermaidTheme } from "./Context";
+
+type MermaidApi = {
+ initialize: (config: { securityLevel: "strict"; startOnLoad: false; theme: MermaidTheme }) => void;
+ render: (diagramId: string, chart: string) => Promise<{ svg: string }>;
+};
+
+let mermaidModulePromise: Promise<MermaidApi> | undefined;
+
+const getMermaid = async (): Promise<MermaidApi> => {
+ mermaidModulePromise ??= import("mermaid").then(({ default: mermaid }) => mermaid as MermaidApi);
+
+ return mermaidModulePromise;
+};
+
+export const MermaidProvider = ({ children }: PropsWithChildren) => {
+ const { colorMode } = useColorMode();
+ const theme: MermaidTheme = colorMode === "dark" ? "dark" : "default";
+ const [initializedTheme, setInitializedTheme] = useState<MermaidTheme | undefined>(undefined);
+ const initializedThemeRef = useRef<MermaidTheme | undefined>(initializedTheme);
+
+ useEffect(() => {
+ initializedThemeRef.current = initializedTheme;
+ }, [initializedTheme]);
+
+ const initializeMermaid = useCallback(async (): Promise<MermaidApi> => {
Review Comment:
We don't really need to call `useCallback` anymore with the latest react compiler
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +134,106 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type MarkdownCodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+const hasDisplayMath = (children: Options["children"]): boolean =>
+ typeof children === "string" && children.includes("$$");
+
+const extractText = (child: ReactNode): string => (typeof child === "string" ? child : "");
+
+const extractTextContent = (children: MarkdownCodeElementProps["children"]): string => {
+ if (Array.isArray(children)) {
+ return children.map(extractText).join("");
+ }
+
+ return extractText(children);
+};
+
+const InlineCodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme) =>
+ ({ children }: { readonly children?: ReactNode }) => {
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<MarkdownCodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
- const match = /language-(?<lang>\w+)/u.exec(className ?? "");
+ const { children: codeChildren, className } = codeElement.props;
+ const match = /language-(?<lang>[-\w]+)/u.exec(className ?? "");
const language = match?.groups?.lang;
+ const childString = extractTextContent(codeChildren).replace(/\n$/u, "");
Review Comment:
```suggestion
const codeText = Array.isArray(codeChildren)
? codeChildren.join("")
: (codeChildren ?? "");
const childString = String(codeText).replace(/\n$/u, "");
```
Then we don't need extractText & extractTextContent
##########
airflow-core/src/airflow/ui/src/context/mermaid/MermaidProvider.tsx:
##########
@@ -0,0 +1,74 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { PropsWithChildren } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+
+import { useColorMode } from "src/context/colorMode";
+
+import { MermaidContext, type MermaidRenderParams, type MermaidTheme } from "./Context";
+
+type MermaidApi = {
+ initialize: (config: { securityLevel: "strict"; startOnLoad: false; theme: MermaidTheme }) => void;
+ render: (diagramId: string, chart: string) => Promise<{ svg: string }>;
+};
+
+let mermaidModulePromise: Promise<MermaidApi> | undefined;
+
+const getMermaid = async (): Promise<MermaidApi> => {
+ mermaidModulePromise ??= import("mermaid").then(({ default: mermaid }) => mermaid as MermaidApi);
+
+ return mermaidModulePromise;
+};
+
+export const MermaidProvider = ({ children }: PropsWithChildren) => {
+ const { colorMode } = useColorMode();
+ const theme: MermaidTheme = colorMode === "dark" ? "dark" : "default";
+ const [initializedTheme, setInitializedTheme] = useState<MermaidTheme | undefined>(undefined);
+ const initializedThemeRef = useRef<MermaidTheme | undefined>(initializedTheme);
+
+ useEffect(() => {
+ initializedThemeRef.current = initializedTheme;
+ }, [initializedTheme]);
Review Comment:
We don't need so much state management. Let's just initialize mermaid during `renderDiagram`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4538691900
@minyeamer — This PR has new commits since the last review requesting changes from `bbovenzi`. Could you address the outstanding review comments and either push a fix or reply in each thread explaining why the feedback doesn't apply? When you believe the threads are resolved, please mark them as resolved and ping the reviewer (`bbovenzi`) — they'll either re-review or hand the PR back to the queue. Thanks!
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238775003
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
I initially switched to the default export based on a GitHub Copilot suggestion to support a wider range of languages without manual registration.
I hadn't fully considered the impact on bundle size at the time. Please let me know your preference, and I will revert this to `PrismLight` with selective registration if needed.
capture:
<img width="779" height="636" alt="image" src="https://github.com/user-attachments/assets/dc96488c-b072-4ab4-8613-c527f490d6fc" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239156697
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
All newly bundled UI dependencies (remark-math, rehype-katex, katex) are MIT-licensed. I will add the corresponding license texts to airflow-core/3rd-party-licenses/ following the existing convention.
##########
airflow-core/src/airflow/ui/src/main.tsx:
##########
@@ -20,6 +20,7 @@ import * as ChakraUI from "@chakra-ui/react";
import * as EmotionReact from "@emotion/react";
import { QueryClientProvider } from "@tanstack/react-query";
import axios, { type AxiosError } from "axios";
+import "katex/dist/katex.min.css";
Review Comment:
I will replace the static `import` with a dynamic one to avoid loading KaTeX CSS as part of the app-wide initial bundle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "Srabasti (via GitHub)" <gi...@apache.org>.
Srabasti commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4437971030
Static checks are failing. Please run prek locally in your branch to fix the errors and then commit to repo. Remember to rebase to ensure all latest commits from community are incorporated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3249381838
##########
airflow-core/src/airflow/ui/src/utils/renderMermaid.ts:
##########
Review Comment:
Applied in 0d96f1f6. I moved Mermaid rendering into a dedicated provider under `src/context/mermaid` and removed the old utility-owned initialization path. Mermaid is still lazy-loaded, but theme initialization now lives in the React lifecycle instead of mutable module state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3249157353
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
Review Comment:
They represent different contracts. Here we are inspecting the `react-markdown` code child element before rendering, specifically its markdown-derived `className` and `children`, not the props of Chakra's rendered `Code` component. I've renamed the local type to `MarkdownCodeElementProps` to make this distinction clearer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4476939938
@minyeamer — There are 1 unresolved review thread on this PR from @bbovenzi. Could you either push a fix or reply in each thread explaining why the feedback doesn't apply? Once you believe the feedback is addressed, mark the thread as resolved so the reviewer isn't re-pinged needlessly. Thanks!
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3349879032
##########
airflow-core/3rd-party-licenses/LICENSE-mermaid.txt:
##########
Review Comment:
Thanks, I added these based on the earlier [feedback](https://github.com/apache/airflow/pull/66809#discussion_r3239003324). I’ll wait for confirmation, and if they’re not needed for the new UI, I’ll remove them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4622731996
@pierrejeambrun Addressed in ea53d864.
Mermaid initialization is now cached per theme in the provider, so `mermaid.initialize()` is not called on every `renderDiagram` and only runs again when the theme changes.
The `ReactMarkdown` spread order change was not intentional, so I restored the previous override behavior by placing `components={components}` before `{...props}`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "parkhojeong (via GitHub)" <gi...@apache.org>.
parkhojeong commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239003324
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
Should we update the third-party license notices for the new bundled UI dependencies added in this PR?
For reference, `airflow-core/LICENSE` already lists bundled UI dependencies such as Chakra UI and hue, with their corresponding license texts under `airflow-core/3rd-party-licenses/`.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
Review Comment:
Is the new behavior intended for every ReactMarkdown usage, or only for Dag Documentation?
This component is shared by Dag/Task docs, dashboard alerts, action note previews, Trigger Dag / FlexibleForm descriptions, and editable markdown previews.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
+ }
+
+ return "";
+};
+
+const CodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme, mermaidTheme: MermaidDiagramProps["theme"]) =>
+ ({ children }: { readonly children?: ReactNode }) => {
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<CodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
- const match = /language-(?<lang>\w+)/u.exec(className ?? "");
+ const { children: codeChildren, className } = codeElement.props;
+ const match = /language-(?<lang>[-\w]+)/u.exec(className ?? "");
const language = match?.groups?.lang;
+ const childString = extractTextContent(codeChildren).replace(/\n$/u, "");
- // Safely extract string content from children
- let childString = "";
-
- if (typeof children === "string") {
- childString = children;
- } else if (Array.isArray(children)) {
- childString = children.filter((child) => typeof child === "string").join("");
+ if (language === "mermaid") {
+ return <MarkdownMermaid chart={childString} fallbackStyle={style} theme={mermaidTheme} />;
}
- return (
- <SyntaxHighlighter language={language ?? "text"} PreTag="div" style={style} wrapLongLines>
- {childString.replace(/\n$/u, "")}
- </SyntaxHighlighter>
- );
+ return <MarkdownCodeBlock language={language} style={style} value={childString} />;
};
+const createMarkdownComponents = ({ mermaidTheme, style }: MarkdownRendererProps): Components => ({
+ // eslint-disable-next-line id-length
+ a: LinkComponent,
+ blockquote: BlockquoteComponent,
+ code: CodeComponent,
+ del: DelComponent,
+ em: EmComponent,
+ h1: makeHeading("h1"),
+ h2: makeHeading("h2"),
+ h3: makeHeading("h3"),
+ h4: makeHeading("h4"),
+ h5: makeHeading("h5"),
+ h6: makeHeading("h6"),
+ hr: HrComponent,
+ img: ImgComponent,
+ li: LiComponent,
+ ol: OlComponent,
+ // eslint-disable-next-line id-length
+ p: PComponent,
+ pre: createPreComponent(style, mermaidTheme),
+ table: TableComponent,
+ tbody: Table.Body,
+ td: Table.Cell,
+ text: TextComponent,
+ th: Table.ColumnHeader,
+ thead: Table.Header,
+ tr: Table.Row,
+ ul: UlComponent,
+});
+
const ReactMarkdown = (props: Options) => {
const { colorMode } = useColorMode();
const style = colorMode === "dark" ? oneDark : oneLight;
-
- const components = {
- // eslint-disable-next-line id-length
- a: LinkComponent,
- blockquote: BlockquoteComponent,
- code: createCodeComponent(style),
- del: DelComponent,
- em: EmComponent,
- h1: makeHeading("h1"),
- h2: makeHeading("h2"),
- h3: makeHeading("h3"),
- h4: makeHeading("h4"),
- h5: makeHeading("h5"),
- h6: makeHeading("h6"),
- hr: HrComponent,
- img: ImgComponent,
- li: LiComponent,
- ol: OlComponent,
- // eslint-disable-next-line id-length
- p: PComponent,
- pre: PreComponent,
- table: TableComponent,
- tbody: Table.Body,
- td: Table.Cell,
- text: TextComponent,
- th: Table.ColumnHeader,
- thead: Table.Header,
- tr: Table.Row,
- ul: UlComponent,
- };
-
- return <ReactMD components={components as Components} {...props} remarkPlugins={[remarkGfm]} skipHtml />;
+ const mermaidTheme = colorMode === "dark" ? "dark" : "default";
+ const components = createMarkdownComponents({ mermaidTheme, style });
+
+ return (
+ <Box alignSelf="stretch" css={markdownContentStyles} maxWidth="100%" minWidth={0} width="100%">
+ <ReactMD
+ components={components}
+ {...props}
+ rehypePlugins={[rehypeKatex]}
+ remarkPlugins={[remarkGfm, remarkMath]}
Review Comment:
This enables remarkMath with its default `singleDollarTextMath: true`, which changes how ordinary dollar text is parsed across all ReactMarkdown usages. For example, `Costs $5 and $10 today.` is parsed as inline math (`5 and `), so the dollar amounts render incorrectly.
<img width="700" height="438" alt="Image" src="https://github.com/user-attachments/assets/75c5c57d-442b-4b62-82e4-c51baece7837" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "parkhojeong (via GitHub)" <gi...@apache.org>.
parkhojeong commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238959086
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
Review Comment:
Could we scope these changes to Dag Documentation to limit the impact?
This component is shared by Dag/Task docs, dashboard alerts, action note previews, Trigger Dag / FlexibleForm descriptions, and editable markdown previews.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3242201254
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
Review Comment:
Yes, please review your AI code first before asking others to review your PR.
We shouldn't be testing third party libraries. We should assume they work, or if they dont, we shouldn't use it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238722887
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export {
+ atomOneDark as oneDark,
+ atomOneLight as oneLight,
+} from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
Review Comment:
Could you share the reasoning behind switching from PrismLight with selective language registration to the default react-syntax-highlighter export here?
This change may impact the initial bundle size, so I’d like to understand the rationale behind this decision :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239157661
##########
airflow-core/src/airflow/ui/src/main.tsx:
##########
@@ -20,6 +20,7 @@ import * as ChakraUI from "@chakra-ui/react";
import * as EmotionReact from "@emotion/react";
import { QueryClientProvider } from "@tanstack/react-query";
import axios, { type AxiosError } from "axios";
+import "katex/dist/katex.min.css";
Review Comment:
All newly bundled UI dependencies (`remark-math`, `rehype-katex`, `katex`) are MIT-licensed. I will add the corresponding license texts to `airflow-core/3rd-party-licenses/` following the existing convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3243021501
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
+ }
+
+ return "";
+};
+
+const CodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme, mermaidTheme: MermaidDiagramProps["theme"]) =>
+ ({ children }: { readonly children?: ReactNode }) => {
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<CodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
- const match = /language-(?<lang>\w+)/u.exec(className ?? "");
+ const { children: codeChildren, className } = codeElement.props;
+ const match = /language-(?<lang>[-\w]+)/u.exec(className ?? "");
const language = match?.groups?.lang;
+ const childString = extractTextContent(codeChildren).replace(/\n$/u, "");
- // Safely extract string content from children
- let childString = "";
-
- if (typeof children === "string") {
- childString = children;
- } else if (Array.isArray(children)) {
- childString = children.filter((child) => typeof child === "string").join("");
+ if (language === "mermaid") {
+ return <MarkdownMermaid chart={childString} fallbackStyle={style} theme={mermaidTheme} />;
}
- return (
- <SyntaxHighlighter language={language ?? "text"} PreTag="div" style={style} wrapLongLines>
- {childString.replace(/\n$/u, "")}
- </SyntaxHighlighter>
- );
+ return <MarkdownCodeBlock language={language} style={style} value={childString} />;
};
+const createMarkdownComponents = ({ mermaidTheme, style }: MarkdownRendererProps): Components => ({
+ // eslint-disable-next-line id-length
+ a: LinkComponent,
+ blockquote: BlockquoteComponent,
+ code: CodeComponent,
+ del: DelComponent,
+ em: EmComponent,
+ h1: makeHeading("h1"),
+ h2: makeHeading("h2"),
+ h3: makeHeading("h3"),
+ h4: makeHeading("h4"),
+ h5: makeHeading("h5"),
+ h6: makeHeading("h6"),
+ hr: HrComponent,
+ img: ImgComponent,
+ li: LiComponent,
+ ol: OlComponent,
+ // eslint-disable-next-line id-length
+ p: PComponent,
+ pre: createPreComponent(style, mermaidTheme),
+ table: TableComponent,
+ tbody: Table.Body,
+ td: Table.Cell,
+ text: TextComponent,
+ th: Table.ColumnHeader,
+ thead: Table.Header,
+ tr: Table.Row,
+ ul: UlComponent,
+});
+
const ReactMarkdown = (props: Options) => {
const { colorMode } = useColorMode();
const style = colorMode === "dark" ? oneDark : oneLight;
-
- const components = {
- // eslint-disable-next-line id-length
- a: LinkComponent,
- blockquote: BlockquoteComponent,
- code: createCodeComponent(style),
- del: DelComponent,
- em: EmComponent,
- h1: makeHeading("h1"),
- h2: makeHeading("h2"),
- h3: makeHeading("h3"),
- h4: makeHeading("h4"),
- h5: makeHeading("h5"),
- h6: makeHeading("h6"),
- hr: HrComponent,
- img: ImgComponent,
- li: LiComponent,
- ol: OlComponent,
- // eslint-disable-next-line id-length
- p: PComponent,
- pre: PreComponent,
- table: TableComponent,
- tbody: Table.Body,
- td: Table.Cell,
- text: TextComponent,
- th: Table.ColumnHeader,
- thead: Table.Header,
- tr: Table.Row,
- ul: UlComponent,
- };
-
- return <ReactMD components={components as Components} {...props} remarkPlugins={[remarkGfm]} skipHtml />;
+ const mermaidTheme = colorMode === "dark" ? "dark" : "default";
+ const components = createMarkdownComponents({ mermaidTheme, style });
+
+ return (
+ <Box alignSelf="stretch" css={markdownContentStyles} maxWidth="100%" minWidth={0} width="100%">
+ <ReactMD
+ components={components}
+ {...props}
+ rehypePlugins={[rehypeKatex]}
+ remarkPlugins={[remarkGfm, remarkMath]}
Review Comment:
Applied the requested changes in f7a7a97f with lazy-load KaTeX
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3228269099
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,111 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
+ }
+
+ return "";
+};
+
+const CodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme, mermaidTheme: MermaidDiagramProps["theme"]) =>
({
children,
- className,
- inline,
}: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
+ readonly children?: ReactNode;
}) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<CodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
+ const { children: codeChildren, className } = codeElement.props;
const match = /language-(?<lang>\w+)/u.exec(className ?? "");
Review Comment:
Language extraction for fenced code blocks uses `/language-(?<lang>\w+)/`, which will truncate or fail for common hyphenated language identifiers (e.g. `language-bash-session`, `language-objective-c`). This can lead to wrong labels and incorrect highlighting/mermaid detection. Consider expanding the regex to include `-` (and possibly other valid characters) when capturing the language.
##########
airflow-core/src/airflow/ui/src/utils/renderMermaid.ts:
##########
@@ -0,0 +1,48 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import mermaid from "mermaid";
+
+type MermaidTheme = "dark" | "default";
+
+let initializedTheme: MermaidTheme | undefined;
+
+const initializeMermaid = (theme: MermaidTheme) => {
+ if (initializedTheme === theme) {
+ return;
+ }
+
+ mermaid.initialize({ securityLevel: "strict", startOnLoad: false, theme });
+ initializedTheme = theme;
+};
Review Comment:
`renderMermaid.ts` imports `mermaid` eagerly at module load. Since `ReactMarkdown` is used across multiple UI surfaces (not just DAG docs), this likely pulls Mermaid (and its heavy dependency tree) into the main bundle even when no Mermaid blocks are present. Consider switching to a dynamic import inside `renderMermaidDiagram` (or inside the Mermaid block component) so Mermaid code is only loaded when a Mermaid fenced block is actually rendered.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdownBlocks.tsx:
##########
@@ -0,0 +1,188 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Flex, Text } from "@chakra-ui/react";
+import { useEffect, useId, useState, type ReactNode } from "react";
+
+import { LazyClipboard } from "src/components/ui";
+import { renderMermaidDiagram } from "src/utils/renderMermaid";
+import { SyntaxHighlighter, type SyntaxTheme } from "src/utils/syntaxHighlighter";
+
+const MarkdownBlockFrame = ({
+ action,
+ children,
+ label,
+}: {
+ readonly action: ReactNode;
+ readonly children: ReactNode;
+ readonly label: string;
+}) => (
+ <Box maxWidth="100%" minWidth={0} my={3} width="100%">
+ <Flex
+ alignItems="center"
+ bg="bg.muted"
+ borderColor="border.emphasized"
+ borderTopRadius="md"
+ borderWidth="1px"
+ justifyContent="space-between"
+ minH={8}
+ px={3}
+ py={1}
+ >
+ <Text color="fg.muted" fontFamily="mono" fontSize="xs" lineHeight="short">
+ {label}
+ </Text>
+ {action}
+ </Flex>
+ <Box
+ borderBottomRadius="md"
+ borderColor="border.emphasized"
+ borderTopWidth={0}
+ borderWidth="1px"
+ maxWidth="100%"
+ minWidth={0}
+ overflow="hidden"
+ >
+ {children}
+ </Box>
+ </Box>
+);
+
+export const MarkdownCodeBlock = ({
+ language,
+ style,
+ value,
+}: {
+ readonly language?: string;
+ readonly style: SyntaxTheme;
+ readonly value: string;
+}) => {
+ const languageLabel = language ?? "text";
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label="Copy code block"
+ data-testid="markdown-copy-button"
+ getValue={() => value}
+ />
+ }
+ label={languageLabel}
+ >
+ <Box data-testid="markdown-code-scroll-area" maxWidth="100%" minWidth={0} overflowX="auto" overflowY="hidden" width="100%">
+ <Box data-testid="markdown-code-content" display="inline-block" minWidth="100%">
+ <SyntaxHighlighter
+ codeTagProps={{ style: { overflowWrap: "normal", whiteSpace: "pre", wordBreak: "normal" } }}
+ customStyle={{ borderRadius: 0, margin: 0, width: "max-content" }}
+ language={languageLabel}
+ lineNumberStyle={{ minWidth: "2.5em", opacity: 0.6, paddingRight: "1em" }}
+ PreTag="div"
+ showLineNumbers
+ style={style}
+ >
+ {value}
+ </SyntaxHighlighter>
+ </Box>
+ </Box>
+ </MarkdownBlockFrame>
+ );
+};
+
+export const MarkdownMermaid = ({
+ chart,
+ fallbackStyle,
+ theme,
+}: {
+ readonly chart: string;
+ readonly fallbackStyle: SyntaxTheme;
+ readonly theme: "dark" | "default";
+}) => {
+ const diagramId = useId().replaceAll(":", "");
+ const [error, setError] = useState(false);
+ const [svg, setSvg] = useState<string>();
+
+ useEffect(() => {
+ let cancelled = false;
+
+ const renderDiagram = async () => {
+ try {
+ const renderedSvg = await renderMermaidDiagram({
+ chart,
+ diagramId: `markdown-mermaid-${diagramId}`,
+ theme,
+ });
+
+ if (!cancelled) {
+ setSvg(renderedSvg);
+ setError(false);
+ }
+ } catch {
+ if (!cancelled) {
+ setError(true);
+ setSvg(undefined);
+ }
+ }
+ };
+
+ void renderDiagram();
+
+ return () => {
+ cancelled = true;
+ };
+ }, [chart, diagramId, theme]);
+
+ if (error) {
+ return <MarkdownCodeBlock language="mermaid" style={fallbackStyle} value={chart} />;
+ }
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label="Copy Mermaid source"
+ data-testid="markdown-mermaid-copy-button"
+ getValue={() => chart}
+ />
+ }
+ label="mermaid"
+ >
+ <Box maxWidth="100%" minHeight="8rem" minWidth={0} overflow="hidden" p={3} width="100%">
+ {svg === undefined ? (
+ <Text color="fg.muted" fontSize="sm">
+ Rendering diagram...
+ </Text>
+ ) : (
+ <Box
+ css={{
+ "& svg": {
+ display: "block",
+ height: "auto",
+ marginInline: "auto",
+ maxWidth: "100%",
+ width: "100%",
+ },
+ }}
+ dangerouslySetInnerHTML={{ __html: svg }}
+ data-testid="markdown-mermaid-diagram"
+ />
Review Comment:
This renders Mermaid output via `dangerouslySetInnerHTML` using SVG generated from `doc_md` content. Even with `securityLevel: "strict"`, this is still an XSS-sensitive sink and relies entirely on Mermaid's sanitization behavior. Please add an explicit sanitization step (or a defensive allowlist check) before injecting, and/or add a regression test that ensures the generated SVG cannot contain script/event-handler attributes.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdownBlocks.tsx:
##########
@@ -0,0 +1,188 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Flex, Text } from "@chakra-ui/react";
+import { useEffect, useId, useState, type ReactNode } from "react";
+
+import { LazyClipboard } from "src/components/ui";
+import { renderMermaidDiagram } from "src/utils/renderMermaid";
+import { SyntaxHighlighter, type SyntaxTheme } from "src/utils/syntaxHighlighter";
+
+const MarkdownBlockFrame = ({
+ action,
+ children,
+ label,
+}: {
+ readonly action: ReactNode;
+ readonly children: ReactNode;
+ readonly label: string;
+}) => (
+ <Box maxWidth="100%" minWidth={0} my={3} width="100%">
+ <Flex
+ alignItems="center"
+ bg="bg.muted"
+ borderColor="border.emphasized"
+ borderTopRadius="md"
+ borderWidth="1px"
+ justifyContent="space-between"
+ minH={8}
+ px={3}
+ py={1}
+ >
+ <Text color="fg.muted" fontFamily="mono" fontSize="xs" lineHeight="short">
+ {label}
+ </Text>
+ {action}
+ </Flex>
+ <Box
+ borderBottomRadius="md"
+ borderColor="border.emphasized"
+ borderTopWidth={0}
+ borderWidth="1px"
+ maxWidth="100%"
+ minWidth={0}
+ overflow="hidden"
+ >
+ {children}
+ </Box>
+ </Box>
+);
+
+export const MarkdownCodeBlock = ({
+ language,
+ style,
+ value,
+}: {
+ readonly language?: string;
+ readonly style: SyntaxTheme;
+ readonly value: string;
+}) => {
+ const languageLabel = language ?? "text";
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label="Copy code block"
+ data-testid="markdown-copy-button"
+ getValue={() => value}
+ />
Review Comment:
New user-visible strings are hard-coded (e.g. the copy button aria-label). The UI generally routes aria-label/title text through i18n (see other `LazyClipboard` usages). Please use the translation mechanism here as well so these labels are localizable and consistent.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdownBlocks.tsx:
##########
@@ -0,0 +1,188 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Flex, Text } from "@chakra-ui/react";
+import { useEffect, useId, useState, type ReactNode } from "react";
+
+import { LazyClipboard } from "src/components/ui";
+import { renderMermaidDiagram } from "src/utils/renderMermaid";
+import { SyntaxHighlighter, type SyntaxTheme } from "src/utils/syntaxHighlighter";
+
+const MarkdownBlockFrame = ({
+ action,
+ children,
+ label,
+}: {
+ readonly action: ReactNode;
+ readonly children: ReactNode;
+ readonly label: string;
+}) => (
+ <Box maxWidth="100%" minWidth={0} my={3} width="100%">
+ <Flex
+ alignItems="center"
+ bg="bg.muted"
+ borderColor="border.emphasized"
+ borderTopRadius="md"
+ borderWidth="1px"
+ justifyContent="space-between"
+ minH={8}
+ px={3}
+ py={1}
+ >
+ <Text color="fg.muted" fontFamily="mono" fontSize="xs" lineHeight="short">
+ {label}
+ </Text>
+ {action}
+ </Flex>
+ <Box
+ borderBottomRadius="md"
+ borderColor="border.emphasized"
+ borderTopWidth={0}
+ borderWidth="1px"
+ maxWidth="100%"
+ minWidth={0}
+ overflow="hidden"
+ >
+ {children}
+ </Box>
+ </Box>
+);
+
+export const MarkdownCodeBlock = ({
+ language,
+ style,
+ value,
+}: {
+ readonly language?: string;
+ readonly style: SyntaxTheme;
+ readonly value: string;
+}) => {
+ const languageLabel = language ?? "text";
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label="Copy code block"
+ data-testid="markdown-copy-button"
+ getValue={() => value}
+ />
+ }
+ label={languageLabel}
+ >
+ <Box data-testid="markdown-code-scroll-area" maxWidth="100%" minWidth={0} overflowX="auto" overflowY="hidden" width="100%">
+ <Box data-testid="markdown-code-content" display="inline-block" minWidth="100%">
+ <SyntaxHighlighter
+ codeTagProps={{ style: { overflowWrap: "normal", whiteSpace: "pre", wordBreak: "normal" } }}
+ customStyle={{ borderRadius: 0, margin: 0, width: "max-content" }}
+ language={languageLabel}
+ lineNumberStyle={{ minWidth: "2.5em", opacity: 0.6, paddingRight: "1em" }}
+ PreTag="div"
+ showLineNumbers
+ style={style}
+ >
+ {value}
+ </SyntaxHighlighter>
+ </Box>
+ </Box>
+ </MarkdownBlockFrame>
+ );
+};
+
+export const MarkdownMermaid = ({
+ chart,
+ fallbackStyle,
+ theme,
+}: {
+ readonly chart: string;
+ readonly fallbackStyle: SyntaxTheme;
+ readonly theme: "dark" | "default";
+}) => {
+ const diagramId = useId().replaceAll(":", "");
+ const [error, setError] = useState(false);
+ const [svg, setSvg] = useState<string>();
+
+ useEffect(() => {
+ let cancelled = false;
+
+ const renderDiagram = async () => {
+ try {
+ const renderedSvg = await renderMermaidDiagram({
+ chart,
+ diagramId: `markdown-mermaid-${diagramId}`,
+ theme,
+ });
+
+ if (!cancelled) {
+ setSvg(renderedSvg);
+ setError(false);
+ }
+ } catch {
+ if (!cancelled) {
+ setError(true);
+ setSvg(undefined);
+ }
+ }
+ };
+
+ void renderDiagram();
+
+ return () => {
+ cancelled = true;
+ };
+ }, [chart, diagramId, theme]);
+
+ if (error) {
+ return <MarkdownCodeBlock language="mermaid" style={fallbackStyle} value={chart} />;
+ }
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label="Copy Mermaid source"
+ data-testid="markdown-mermaid-copy-button"
+ getValue={() => chart}
+ />
+ }
+ label="mermaid"
+ >
+ <Box maxWidth="100%" minHeight="8rem" minWidth={0} overflow="hidden" p={3} width="100%">
+ {svg === undefined ? (
+ <Text color="fg.muted" fontSize="sm">
+ Rendering diagram...
+ </Text>
+ ) : (
Review Comment:
The Mermaid block UI hard-codes user-visible strings (copy button aria-label and the loading message). Please route these through i18n (consistent with other UI components) so they can be localized, and update the tests accordingly.
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
@@ -0,0 +1,153 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import "@testing-library/jest-dom";
Review Comment:
`testsSetup.ts` already imports `@testing-library/jest-dom/vitest`, so importing `@testing-library/jest-dom` again here is redundant and may register matchers in a non-Vitest-specific way. Consider removing this import and relying on the global test setup.
##########
airflow-core/src/airflow/ui/src/utils/syntaxHighlighter.ts:
##########
@@ -16,20 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
-import bash from "react-syntax-highlighter/dist/esm/languages/prism/bash";
-import json from "react-syntax-highlighter/dist/esm/languages/prism/json";
-import python from "react-syntax-highlighter/dist/esm/languages/prism/python";
-import sql from "react-syntax-highlighter/dist/esm/languages/prism/sql";
-import yaml from "react-syntax-highlighter/dist/esm/languages/prism/yaml";
+import type { CSSProperties } from "react";
-// Register all supported languages once
-SyntaxHighlighter.registerLanguage("python", python);
-SyntaxHighlighter.registerLanguage("json", json);
-SyntaxHighlighter.registerLanguage("yaml", yaml);
-SyntaxHighlighter.registerLanguage("sql", sql);
-SyntaxHighlighter.registerLanguage("bash", bash);
+export { atomOneDark as oneDark, atomOneLight as oneLight } from "react-syntax-highlighter/dist/esm/styles/hljs";
-export { oneDark, oneLight } from "react-syntax-highlighter/dist/esm/styles/prism";
+export { default as SyntaxHighlighter } from "react-syntax-highlighter";
-export { PrismLight as SyntaxHighlighter } from "react-syntax-highlighter";
+export type SyntaxTheme = Record<string, CSSProperties>;
Review Comment:
Switching from `PrismLight` with explicit language registration to the default `react-syntax-highlighter` export will typically bundle a much larger highlighter/language set. Given this helper is used across the UI, consider using a light/async build (and/or selective language registration) to keep bundle size and initial load time under control while still supporting the needed languages.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3238895900
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
Review Comment:
Personally, I think this part may not strictly need to be tested, but if we want to keep the tests, would it be okay to reduce them to a minimal set of smoke tests, such as the mermaid error fallback and basic math integration, and drop the rest?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3239154704
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx:
##########
@@ -106,81 +122,107 @@ const UlComponent = ({ children }: PropsWithChildren) => (
</List.Root>
);
-// Factory function for the code component that needs style
-const createCodeComponent =
- (style: typeof oneDark | typeof oneLight) =>
- ({
- children,
- className,
- inline,
- }: {
- readonly children: ReactNode;
- readonly className?: string;
- readonly inline?: boolean;
- }) => {
- if (inline) {
- return (
- <Code display="inline" p={2}>
- {children}
- </Code>
- );
+type CodeElementProps = {
+ readonly children?: ReactNode;
+ readonly className?: string;
+};
+
+type MermaidDiagramProps = {
+ readonly chart: string;
+ readonly theme: "dark" | "default";
+};
+
+type MarkdownRendererProps = {
+ readonly mermaidTheme: MermaidDiagramProps["theme"];
+ readonly style: SyntaxTheme;
+};
+
+const extractTextContent = (children: CodeElementProps["children"]): string => {
+ if (typeof children === "number" || typeof children === "string") {
+ return String(children);
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => (typeof child === "number" || typeof child === "string" ? String(child) : ""))
+ .join("");
+ }
+
+ return "";
+};
+
+const CodeComponent = ({ children }: PropsWithChildren) => <Code display="inline">{children}</Code>;
+
+// Factory function for the pre component that needs style
+const createPreComponent =
+ (style: SyntaxTheme, mermaidTheme: MermaidDiagramProps["theme"]) =>
+ ({ children }: { readonly children?: ReactNode }) => {
+ const [codeElement] = Children.toArray(children);
+
+ if (!isValidElement<CodeElementProps>(codeElement)) {
+ return <Box my={3}>{children}</Box>;
}
// Extract language from className (format: "language-python")
- const match = /language-(?<lang>\w+)/u.exec(className ?? "");
+ const { children: codeChildren, className } = codeElement.props;
+ const match = /language-(?<lang>[-\w]+)/u.exec(className ?? "");
const language = match?.groups?.lang;
+ const childString = extractTextContent(codeChildren).replace(/\n$/u, "");
- // Safely extract string content from children
- let childString = "";
-
- if (typeof children === "string") {
- childString = children;
- } else if (Array.isArray(children)) {
- childString = children.filter((child) => typeof child === "string").join("");
+ if (language === "mermaid") {
+ return <MarkdownMermaid chart={childString} fallbackStyle={style} theme={mermaidTheme} />;
}
- return (
- <SyntaxHighlighter language={language ?? "text"} PreTag="div" style={style} wrapLongLines>
- {childString.replace(/\n$/u, "")}
- </SyntaxHighlighter>
- );
+ return <MarkdownCodeBlock language={language} style={style} value={childString} />;
};
+const createMarkdownComponents = ({ mermaidTheme, style }: MarkdownRendererProps): Components => ({
+ // eslint-disable-next-line id-length
+ a: LinkComponent,
+ blockquote: BlockquoteComponent,
+ code: CodeComponent,
+ del: DelComponent,
+ em: EmComponent,
+ h1: makeHeading("h1"),
+ h2: makeHeading("h2"),
+ h3: makeHeading("h3"),
+ h4: makeHeading("h4"),
+ h5: makeHeading("h5"),
+ h6: makeHeading("h6"),
+ hr: HrComponent,
+ img: ImgComponent,
+ li: LiComponent,
+ ol: OlComponent,
+ // eslint-disable-next-line id-length
+ p: PComponent,
+ pre: createPreComponent(style, mermaidTheme),
+ table: TableComponent,
+ tbody: Table.Body,
+ td: Table.Cell,
+ text: TextComponent,
+ th: Table.ColumnHeader,
+ thead: Table.Header,
+ tr: Table.Row,
+ ul: UlComponent,
+});
+
const ReactMarkdown = (props: Options) => {
const { colorMode } = useColorMode();
const style = colorMode === "dark" ? oneDark : oneLight;
-
- const components = {
- // eslint-disable-next-line id-length
- a: LinkComponent,
- blockquote: BlockquoteComponent,
- code: createCodeComponent(style),
- del: DelComponent,
- em: EmComponent,
- h1: makeHeading("h1"),
- h2: makeHeading("h2"),
- h3: makeHeading("h3"),
- h4: makeHeading("h4"),
- h5: makeHeading("h5"),
- h6: makeHeading("h6"),
- hr: HrComponent,
- img: ImgComponent,
- li: LiComponent,
- ol: OlComponent,
- // eslint-disable-next-line id-length
- p: PComponent,
- pre: PreComponent,
- table: TableComponent,
- tbody: Table.Body,
- td: Table.Cell,
- text: TextComponent,
- th: Table.ColumnHeader,
- thead: Table.Header,
- tr: Table.Row,
- ul: UlComponent,
- };
-
- return <ReactMD components={components as Components} {...props} remarkPlugins={[remarkGfm]} skipHtml />;
+ const mermaidTheme = colorMode === "dark" ? "dark" : "default";
+ const components = createMarkdownComponents({ mermaidTheme, style });
+
+ return (
+ <Box alignSelf="stretch" css={markdownContentStyles} maxWidth="100%" minWidth={0} width="100%">
+ <ReactMD
+ components={components}
+ {...props}
+ rehypePlugins={[rehypeKatex]}
+ remarkPlugins={[remarkGfm, remarkMath]}
Review Comment:
Thank you for pointing this out. this was something I hadn't fully considered.
I will set `singleDollarTextMath: false` ([docs](https://unifiedjs.com/explore/package/remark-math/#fields)) to prevent single `$` from being parsed as inline math. Block math with `$$...$$` will still work as expected.
Looking ahead, re-enabling single-dollar inline math would require existing documents to escape bare dollar signs (e.g., `\$5`). I think it is worth opening a community discussion or vote on whether that trade-off is acceptable before reintroducing it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #66809:
URL: https://github.com/apache/airflow/pull/66809
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3468714591
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdownBlocks.tsx:
##########
@@ -0,0 +1,221 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Flex, Spinner, Text } from "@chakra-ui/react";
+import { useEffect, useId, useState, type ReactNode } from "react";
+import { useTranslation } from "react-i18next";
+
+import { LazyClipboard } from "src/components/ui";
+import { useColorMode } from "src/context/colorMode";
+import { renderMermaidDiagram } from "src/utils/renderMermaid";
+import { SyntaxHighlighter, type SyntaxTheme } from "src/utils/syntaxHighlighter";
+
+const MarkdownBlockFrame = ({
+ action,
+ children,
+ label,
+}: {
+ readonly action: ReactNode;
+ readonly children: ReactNode;
+ readonly label: string;
+}) => (
+ <Box maxWidth="100%" minWidth={0} my={3} width="100%">
+ <Flex
+ alignItems="center"
+ bg="bg.muted"
+ borderColor="border.emphasized"
+ borderTopRadius="md"
+ borderWidth="1px"
+ justifyContent="space-between"
+ minH={8}
+ px={3}
+ py={1}
+ >
+ <Text color="fg.muted" fontFamily="mono" fontSize="xs" lineHeight="short">
+ {label}
+ </Text>
+ {action}
+ </Flex>
+ <Box
+ borderBottomRadius="md"
+ borderColor="border.emphasized"
+ borderTopWidth={0}
+ borderWidth="1px"
+ maxWidth="100%"
+ minWidth={0}
+ overflow="hidden"
+ >
+ {children}
+ </Box>
+ </Box>
+);
+
+export const MarkdownCodeBlock = ({
+ language,
+ style,
+ value,
+}: {
+ readonly language?: string;
+ readonly style: SyntaxTheme;
+ readonly value: string;
+}) => {
+ const { t: translate } = useTranslation("components");
+ const codeBlockStyle = style['pre[class*="language-"]'];
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label={translate("clipboard.copy")}
+ data-testid="markdown-copy-button"
+ getValue={() => value}
+ title={translate("clipboard.copy")}
+ />
+ }
+ label={language ?? "text"}
+ >
+ <Box
+ css={{ ...codeBlockStyle, borderRadius: 0, margin: 0 }}
+ data-testid="markdown-code-scroll-area"
+ maxWidth="100%"
+ minWidth={0}
+ overflowX="auto"
+ overflowY="hidden"
+ width="100%"
+ >
+ <Box data-testid="markdown-code-content" display="inline-block" minWidth="100%">
+ <SyntaxHighlighter
+ codeTagProps={{
+ style: {
+ background: "transparent",
+ overflowWrap: "normal",
+ whiteSpace: "pre",
+ wordBreak: "normal",
+ },
+ }}
+ customStyle={{
+ background: "transparent",
+ borderRadius: 0,
+ margin: 0,
+ padding: 0,
+ width: "max-content",
+ }}
+ language={language}
+ lineNumberStyle={{ minWidth: "2.5em", opacity: 0.6, paddingRight: "1em" }}
+ PreTag="div"
+ showLineNumbers
+ style={style}
+ >
+ {value}
+ </SyntaxHighlighter>
+ </Box>
+ </Box>
+ </MarkdownBlockFrame>
+ );
+};
+
+export const MarkdownMermaid = ({
+ chart,
+ fallbackStyle,
+}: {
+ readonly chart: string;
+ readonly fallbackStyle: SyntaxTheme;
+}) => {
+ const { t: translate } = useTranslation("components");
+ const { colorMode } = useColorMode();
+ const diagramId = useId().replaceAll(":", "");
+ const [error, setError] = useState(false);
+ const [svg, setSvg] = useState<string>();
+ const theme = colorMode === "dark" ? "dark" : "default";
+
+ useEffect(() => {
+ let cancelled = false;
+
+ const renderMermaid = async () => {
+ try {
+ const renderedSvg = await renderMermaidDiagram({
+ chart,
+ diagramId: `markdown-mermaid-${diagramId}`,
+ theme,
+ });
+
+ if (!cancelled) {
+ setSvg(renderedSvg);
+ setError(false);
+ }
+ } catch {
+ if (!cancelled) {
+ setError(true);
+ setSvg(undefined);
+ }
+ }
+ };
+
+ void renderMermaid();
+
+ return () => {
+ cancelled = true;
+ };
+ }, [chart, diagramId, theme]);
+
+ if (error) {
+ return <MarkdownCodeBlock language="mermaid" style={fallbackStyle} value={chart} />;
+ }
+
+ return (
+ <MarkdownBlockFrame
+ action={
+ <LazyClipboard
+ aria-label={translate("clipboard.copy")}
+ data-testid="markdown-mermaid-copy-button"
+ getValue={() => chart}
+ title={translate("clipboard.copy")}
+ />
+ }
+ label="mermaid"
+ >
+ <Box maxWidth="100%" minHeight="8rem" minWidth={0} overflow="hidden" p={3} width="100%">
+ {svg === undefined ? (
+ <Box
+ alignItems="center"
+ color="fg.muted"
+ data-testid="markdown-mermaid-loading"
+ display="inline-flex"
+ minHeight="2rem"
+ >
+ <Spinner size="sm" />
+ </Box>
+ ) : (
+ <Box
+ css={{
+ "& svg": {
+ display: "block",
+ height: "auto",
+ marginInline: "auto",
+ maxWidth: "100%",
+ width: "100%",
+ },
+ }}
+ dangerouslySetInnerHTML={{ __html: svg }}
Review Comment:
```suggestion
/* Trusting mermaid's strict-mode sanitizer */
dangerouslySetInnerHTML={{ __html: svg }}
```
##########
airflow-core/src/airflow/example_dags/tutorial.py:
##########
@@ -87,16 +87,35 @@
# [START documentation]
t1.doc_md = textwrap.dedent(
"""\
- #### Task Documentation
- You can document your task using the attributes `doc_md` (markdown),
- `doc` (plain text), `doc_rst`, `doc_json`, `doc_yaml` which gets
- rendered in the UI's Task Instance Details page.
- 
- **Image Credit:** Randall Munroe, [XKCD](https://xkcd.com/license.html)
+ #### Print the current date
+
+ This task runs `date` by using the `bash_command` argument on `BashOperator`.
+ In the Task Instance Details page, Airflow renders this documentation from
+ the task's `doc_md` field. After this task succeeds, Airflow can run both
+ downstream tasks: `sleep` and `templated`.
+
+ ```bash
+ date
+ ```
+
+ Display math is rendered with KaTeX. This tutorial starts one task and then
+ branches into two downstream tasks:
+
+ $$
Review Comment:
`$$` fence is ambiguous and could wrongly detect a `math` expression.
Is is possible to switch to a
\`\`\`math
\`\`\`
fence instead? to be consistant with code fense?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3243035555
##########
airflow-core/src/airflow/ui/package.json:
##########
Review Comment:
I updated `airflow-core/LICENSE` plus the corresponding files under `airflow-core/3rd-party-licenses/` following the existing convention. ref: dbf3a19c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on code in PR #66809:
URL: https://github.com/apache/airflow/pull/66809#discussion_r3243012896
##########
airflow-core/src/airflow/ui/src/components/ReactMarkdown.test.tsx:
##########
Review Comment:
Applied the requested changes in c952f382
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "minyeamer (via GitHub)" <gi...@apache.org>.
minyeamer commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4556231014
@bbovenzi Thanks for the review. Addressed all three:
- **`useCallback`**: Understood that `babel-plugin-react-compiler` already handles memoization at build time via `reactCompilerPreset()`, so removed `useCallback` from `initializeMermaid` and `renderDiagram`, and dropped the `useMemo` wrapping the context value. (f328e09d)
- **State management**: Understood that `mermaid.initialize()` is cheap enough to call on every `renderDiagram` invocation, so removed the `useState` / `useRef` / `useEffect` tracking chain and the separate `initializeMermaid` function entirely. (f328e09d)
- **`extractTextContent`**: Understood that both helpers were only used in one place, so inlined the logic as an `Array.isArray` check with `String()` conversion and removed both functions. (b7cfacd9)
---
Drafted-by: Claude Code (claude-sonnet-4-6); reviewed by @minyeamer before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UI: Improve doc_md rendering in Dag Documentation [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #66809:
URL: https://github.com/apache/airflow/pull/66809#issuecomment-4702920991
That improved markdown also including mermaid is really cool! But after merge nobody will know (except when reading the code). So can you please (1) ass a small hint to docs what is supported (proposal: (a) airflow-core/docs/core-concepts/dags.rst and (b) airflow-core/docs/tutorial/fundamentals.rst) and add an extended example also to one of the example dags (e.g. airflow-core/src/airflow/example_dags/tutorial.py)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org