Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a markdown preview feature to the task creation/editing form by adapting PR #67 for Rails 7. The main change involves replacing the deprecated Webpack-based javascript_pack_tag with inline JavaScript to enable real-time markdown preview functionality during task description input.
Key Changes:
- Replaced the single description textarea with a tabbed interface (Write/Preview tabs)
- Added inline JavaScript to fetch and render markdown preview from a backend API endpoint
- Transitioned from Webpack-based JavaScript to inline script approach
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| document.getElementById('preview').onchange=function(){ | ||
| const text = document.getElementById("write-area").value | ||
| const preview = document.getElementById("preview-area") | ||
| const obj = {text: text}; | ||
| const method = "POST"; | ||
| const body = JSON.stringify(obj); | ||
| const headers = { | ||
| 'Accept': 'application/json', | ||
| 'Content-Type': 'application/json' | ||
| }; | ||
| fetch("/documents/api_markdown", {method, headers, body}).then((res)=> res.json()).then((obj) => {preview.innerHTML=obj.text}).catch(console.error);} |
There was a problem hiding this comment.
The preview innerHTML is set directly from the API response without sanitization, which could lead to XSS vulnerabilities if the markdown rendering doesn't properly escape HTML. Ensure the backend sanitizes the output or use textContent/safer DOM manipulation methods.
| document.getElementById('preview').onchange=function(){ | ||
| const text = document.getElementById("write-area").value | ||
| const preview = document.getElementById("preview-area") | ||
| const obj = {text: text}; | ||
| const method = "POST"; | ||
| const body = JSON.stringify(obj); | ||
| const headers = { | ||
| 'Accept': 'application/json', | ||
| 'Content-Type': 'application/json' | ||
| }; | ||
| fetch("/documents/api_markdown", {method, headers, body}).then((res)=> res.json()).then((obj) => {preview.innerHTML=obj.text}).catch(console.error);} |
There was a problem hiding this comment.
[nitpick] The entire inline script lacks proper indentation and formatting. The function body should be properly indented for readability and maintainability.
| document.getElementById('preview').onchange=function(){ | |
| const text = document.getElementById("write-area").value | |
| const preview = document.getElementById("preview-area") | |
| const obj = {text: text}; | |
| const method = "POST"; | |
| const body = JSON.stringify(obj); | |
| const headers = { | |
| 'Accept': 'application/json', | |
| 'Content-Type': 'application/json' | |
| }; | |
| fetch("/documents/api_markdown", {method, headers, body}).then((res)=> res.json()).then((obj) => {preview.innerHTML=obj.text}).catch(console.error);} | |
| document.getElementById('preview').onchange = function() { | |
| const text = document.getElementById("write-area").value; | |
| const preview = document.getElementById("preview-area"); | |
| const obj = { text: text }; | |
| const method = "POST"; | |
| const body = JSON.stringify(obj); | |
| const headers = { | |
| 'Accept': 'application/json', | |
| 'Content-Type': 'application/json' | |
| }; | |
| fetch("/documents/api_markdown", { method, headers, body }) | |
| .then((res) => res.json()) | |
| .then((obj) => { preview.innerHTML = obj.text; }) | |
| .catch(console.error); | |
| } |
| const text = document.getElementById("write-area").value | ||
| const preview = document.getElementById("preview-area") |
There was a problem hiding this comment.
[nitpick] Missing semicolons at the end of statements on lines 123, 124, and 125. While JavaScript allows this through ASI (Automatic Semicolon Insertion), explicit semicolons are recommended for consistency, especially since semicolons are used elsewhere in the script (lines 126-132).
| const text = document.getElementById("write-area").value | |
| const preview = document.getElementById("preview-area") | |
| const text = document.getElementById("write-area").value; | |
| const preview = document.getElementById("preview-area"); |
| 'Accept': 'application/json', | ||
| 'Content-Type': 'application/json' | ||
| }; | ||
| fetch("/documents/api_markdown", {method, headers, body}).then((res)=> res.json()).then((obj) => {preview.innerHTML=obj.text}).catch(console.error);} |
There was a problem hiding this comment.
[nitpick] The promise chain should be formatted across multiple lines for better readability. Additionally, the error handling with console.error doesn't provide user feedback when the preview fails to load.
| fetch("/documents/api_markdown", {method, headers, body}).then((res)=> res.json()).then((obj) => {preview.innerHTML=obj.text}).catch(console.error);} | |
| fetch("/documents/api_markdown", { method, headers, body }) | |
| .then((res) => res.json()) | |
| .then((obj) => { | |
| preview.innerHTML = obj.text; | |
| }) | |
| .catch((error) => { | |
| console.error(error); | |
| preview.innerHTML = '<div class="alert alert-danger">プレビューの読み込みに失敗しました。</div>'; | |
| }); | |
| } |
app/views/tasks/_form.html.erb
Outdated
| <input id="preview" type="radio" name="tab_item"> | ||
| <label class="tab_item" for="preview">Preview</label> | ||
| <div class="tab_content" id="write_content"> | ||
| <%= form.text_area :description, class: "border-2 w-full", rows: 10, id: "write-area", placeholder: "内容を入力してください" %> |
There was a problem hiding this comment.
The CSS classes 'border-2' and 'w-full' appear to be Tailwind CSS utility classes, but the rest of the form uses Bootstrap classes (e.g., 'form-control', 'btn btn-danger'). This inconsistency could lead to styling conflicts or maintenance issues. Consider using consistent CSS framework classes throughout the form.
| <%= form.text_area :description, class: "border-2 w-full", rows: 10, id: "write-area", placeholder: "内容を入力してください" %> | |
| <%= form.text_area :description, class: "form-control", rows: 10, id: "write-area", placeholder: "内容を入力してください" %> |
| </div> | ||
| <div class="tab_content" id="preview_content"> | ||
| <div style="height: 300px" class="overflow-y-scroll"> | ||
| <div id="preview-area" class="markdown-body p-12 list-style-none"> |
There was a problem hiding this comment.
The 'p-12' class appears to be a Tailwind CSS utility class (padding: 3rem), while 'list-style-none' is not standard Tailwind syntax (should be 'list-none'). This mixing of frameworks and potential class name errors could cause unexpected styling behavior.
| <div id="preview-area" class="markdown-body p-12 list-style-none"> | |
| <div id="preview-area" class="markdown-body p-12 list-none"> |
Rails 7 では `javascript_pack_tag` が使えないため,インラインスクリプトで記述するように変更した.
内容
javascript_pack_tagをインラインスクリプトを用いる方式に変更した.タスクのプレビュー表示