feat: Add markdown editor for description#40
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds markdown editing and rendering functionality to description fields in interventions and equipments, replacing plain textarea inputs with a rich markdown editor. The implementation includes two new shared components and updates to both the form and display pages for interventions and equipments.
Key changes:
- Introduces
MarkdownEditorcomponent with toolbar for bold, italic, strikethrough, and link formatting - Introduces
MarkdownRendercomponent for displaying markdown content on show pages - Integrates
@uiw/react-md-editorlibrary for the editing experience with live preview
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/components/markdown-editor.tsx | New component providing markdown editing with sanitized preview |
| src/shared/components/markdown-render.tsx | New component for rendering markdown content on read-only pages |
| src/interventions/components/intervention-form.tsx | Replaced Input.TextArea with MarkdownEditor for description field |
| src/interventions/pages/show.tsx | Replaced plain text display with MarkdownRender component, moved description section for better layout |
| src/equipments/components/equipment-form.tsx | Replaced Input.TextArea with MarkdownEditor for description field |
| src/equipments/pages/show.tsx | Replaced plain text display with MarkdownRender component, moved description section for better layout |
| package.json | Added dependencies: @uiw/react-md-editor, react-markdown, rehype-sanitize |
| yarn.lock | Lock file updated with new dependencies and their transitive dependencies |
|
|
||
| interface MarkdownRenderProps { | ||
| content?: string; | ||
| } | ||
|
|
||
| const MarkdownRender: React.FC<MarkdownRenderProps> = ({ content }) => { | ||
| return <ReactMarkdown>{content}</ReactMarkdown>; |
There was a problem hiding this comment.
The MarkdownRender component doesn't apply sanitization when rendering markdown content. While the MarkdownEditor uses rehypeSanitize for its preview, the read-only display should also sanitize content for security.
Consider adding rehype-sanitize to the MarkdownRender component:
import ReactMarkdown from 'react-markdown';
import rehypeSanitize from 'rehype-sanitize';
const MarkdownRender: React.FC<MarkdownRenderProps> = ({ content }) => {
return (
<ReactMarkdown rehypePlugins={[rehypeSanitize]}>
{content || ''}
</ReactMarkdown>
);
};This ensures that any markdown content displayed is sanitized, preventing potential XSS attacks from stored content.
| interface MarkdownRenderProps { | |
| content?: string; | |
| } | |
| const MarkdownRender: React.FC<MarkdownRenderProps> = ({ content }) => { | |
| return <ReactMarkdown>{content}</ReactMarkdown>; | |
| import rehypeSanitize from 'rehype-sanitize'; | |
| interface MarkdownRenderProps { | |
| content?: string; | |
| } | |
| const MarkdownRender: React.FC<MarkdownRenderProps> = ({ content }) => { | |
| return ( | |
| <ReactMarkdown rehypePlugins={[rehypeSanitize]}> | |
| {content} | |
| </ReactMarkdown> | |
| ); |
| "react-dom": "^18.0.0", | ||
| "react-i18next": "^15.6.0", | ||
| "react-router": "^7.6.3" | ||
| "react-markdown": "^10.1.0", |
There was a problem hiding this comment.
The explicit dependency on react-markdown@^10.1.0 may conflict with @uiw/react-md-editor which internally uses react-markdown@~9.0.1 (as seen in yarn.lock line 6787). This could lead to multiple versions of react-markdown being installed.
Consider removing the explicit react-markdown dependency from package.json and rely on the version that comes with @uiw/react-md-editor, or ensure version compatibility. If you need react-markdown@^10.1.0 for the MarkdownRender component separately, document why both versions are needed.
| "react-markdown": "^10.1.0", |
| <MDEditor | ||
| value={value} | ||
| onChange={onChange} | ||
| data-color-mode="light" | ||
| commands={[ | ||
| commands.bold, | ||
| commands.italic, | ||
| commands.strikethrough, | ||
| commands.link, | ||
| ]} | ||
| previewOptions={{ | ||
| rehypePlugins: [[rehypeSanitize]], | ||
| }} | ||
| /> |
There was a problem hiding this comment.
[nitpick] Consider adding an accessible label or aria-label to the MDEditor component to improve accessibility for screen readers. Since this is used within Form.Item, the label is provided by the form, but for standalone usage or better accessibility, you might want to accept an optional id or aria-label prop:
interface MarkdownEditorProps {
value?: string;
onChange?: (value: string | undefined) => void;
id?: string;
}
const MarkdownEditor: React.FC<MarkdownEditorProps> = ({ value, onChange, id }) => {
return (
<MDEditor
id={id}
value={value}
onChange={onChange}
...
/>
);
};| value?: string; | ||
| onChange?: (value: string | undefined) => void; | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] Consider adding JSDoc comments to document:
- What markdown features are supported (bold, italic, strikethrough, links)
- The sanitization being applied via
rehypeSanitize - Any usage examples or limitations
Example:
/**
* A markdown editor component with sanitization support.
*
* Supports the following markdown features:
* - Bold text
* - Italic text
* - Strikethrough
* - Links
*
* Content is sanitized using rehype-sanitize to prevent XSS attacks.
*/| /** | |
| * A markdown editor component with sanitization support. | |
| * | |
| * Supports the following markdown features: | |
| * - Bold text | |
| * - Italic text | |
| * - Strikethrough | |
| * - Links | |
| * | |
| * Content is sanitized using rehype-sanitize to prevent XSS attacks. | |
| * | |
| * Usage example: | |
| * ```tsx | |
| * <MarkdownEditor value={markdown} onChange={setMarkdown} /> | |
| * ``` | |
| * | |
| * Limitations: | |
| * - Only the listed markdown features are supported. | |
| * - Other markdown features (e.g., tables, code blocks) may not be available. | |
| */ |
| interface MarkdownRenderProps { | ||
| content?: string; | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] Consider adding JSDoc comments to document the component's purpose and any limitations:
/**
* Renders markdown content as HTML.
*
* @param content - The markdown string to render. Can be undefined.
*
* @remarks
* This component uses react-markdown to safely parse and render markdown content.
* It's companion to the MarkdownEditor component.
*/| /** | |
| * Renders markdown content as HTML. | |
| * | |
| * @param content - The markdown string to render. Can be undefined. | |
| * | |
| * @remarks | |
| * This component uses react-markdown to safely parse and render markdown content. | |
| * It's companion to the MarkdownEditor component. | |
| */ |
1c1ec8c to
dcabbcb
Compare
Fixes #33