Skip to content

Conversation

@bear-KU
Copy link
Contributor

@bear-KU bear-KU commented Oct 23, 2025

内容

  • タスクの作成,編集中にマークダウンのプレビューが確認できるようにする PR Add task preview #67 をマージできるように修正した.
    • PR が出されていたブランチを取り込んだ
    • main ブランチとのコンフリクトを解消し,rebase した
  • Rails 7 移行時の Webpack 廃止に伴い,以前の PR でプレビューの表示に利用していた javascript_pack_tag をインラインスクリプトを用いる方式に変更した.

タスクのプレビュー表示

Screenshot from 2025-10-23 18-55-20 Screenshot from 2025-10-23 18-55-28

@bear-KU bear-KU requested a review from Copilot October 23, 2025 10:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +123 to +133
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);}
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +123 to +133
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);}
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The entire inline script lacks proper indentation and formatting. The function body should be properly indented for readability and maintainability.

Suggested change
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);
}

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +125
const text = document.getElementById("write-area").value
const preview = document.getElementById("preview-area")
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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).

Suggested change
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");

Copilot uses AI. Check for mistakes.
'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);}
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
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>';
});
}

Copilot uses AI. Check for mistakes.
<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: "内容を入力してください" %>
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
<%= 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: "内容を入力してください" %>

Copilot uses AI. Check for mistakes.
</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">
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
<div id="preview-area" class="markdown-body p-12 list-style-none">
<div id="preview-area" class="markdown-body p-12 list-none">

Copilot uses AI. Check for mistakes.
Rails 7 では `javascript_pack_tag` が使えないため,インラインスクリプトで記述するように変更した.
@Wistlap Wistlap merged commit 17df1f0 into nomlab:main Oct 28, 2025
2 checks passed
@Wistlap Wistlap mentioned this pull request Oct 28, 2025
@bear-KU bear-KU deleted the dev branch November 13, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants