feat: add text editor and move to SWAL2 #46
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a comprehensive refactor and expansion of the application's UI and functionality. The CSS was overhauled for unified layout, theming, and accessibility, with significant improvements to dark mode and component interactivity. The HTML structure was reorganized to support new features, notably a fully-featured Markdown Editor mode with tab management. The JavaScript adds SweetAlert2-based dialogs, refactors mode switching, and implements the Markdown Editor with persistent, draggable tabs, autosave, and keyboard shortcuts. No changes were made to exported or public entity declarations in the HTML or CSS, but several new and updated functions were introduced in the JavaScript. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (HTML/CSS)
participant JS Logic
participant LocalStorage
participant ToastUIEditor
participant SweetAlert2
User->>UI (HTML/CSS): Clicks "Markdown Editor" mode
UI (HTML/CSS)->>JS Logic: Triggers mode switch
JS Logic->>UI (HTML/CSS): Shows editor section, hides others
JS Logic->>LocalStorage: Loads editor global state (tabs, active tab)
JS Logic->>ToastUIEditor: Initializes editor for active tab
User->>UI (HTML/CSS): Adds/Renames/Deletes/Reorders tabs
UI (HTML/CSS)->>JS Logic: Handles tab action
JS Logic->>LocalStorage: Updates/saves editor state
JS Logic->>SweetAlert2: Shows modal for confirm/notify actions
User->>UI (HTML/CSS): Edits content or saves (Ctrl/Cmd+Shift+S)
UI (HTML/CSS)->>JS Logic: Triggers saveEditorContent
JS Logic->>LocalStorage: Persists tab content
JS Logic->>SweetAlert2: Shows save notification
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
index.js (3)
2-6: 🛠️ Refactor suggestion
getActiveMode()ignores three new modesOnly
formatter,compare, andcodegenare detected, so when the UI is in convert, mockgen, or editor modes, the persistedactiveModeis always recorded as"formatter".
Restoring the session will therefore open the wrong section.function getActiveMode() { - if (document.getElementById("formatter-section").style.display !== "none") return "formatter"; - if (document.getElementById("compare-section").style.display !== "none") return "compare"; - if (document.getElementById("codegen-section").style.display !== "none") return "codegen"; - return "formatter"; + const modes = ["formatter","compare","codegen","convert","mockgen","editor"]; + for (const m of modes) { + if (document.getElementById(`${m}-section`).style.display !== "none") { + return m; + } + } + return "formatter"; // sensible fallback }
240-258:⚠️ Potential issueDuplicate “raw-preview” IDs will break DOM queries
#${tabId}-raw-previewis rendered twice: once at lines 240-243 and again at 250-253.
Duplicate IDs are invalid HTML, andquerySelector("#id")will only hit the first instance, confusing copy/preview logic.- <div id="${tabId}-raw-preview" class="preview-section active"> - <button class="copy-button" onclick="copyRawJSON('${tabId}')">Copy</button> - <pre class="raw-json"></pre> - </div> - <div id="${tabId}-tree-preview" class="preview-section"> - <button class="copy-button" onclick="copyRawJSON('${tabId}')">Copy JSON</button> - <div class="tree-view"></div> - </div> + <!-- second block removed – first one already contains copy btn & pre -->
1073-1100:⚠️ Potential issueNull-dereference in
convert()Because of the missing
convert-outputelement (see HTML comment),outputcan benull, and the following lines attemptoutput.textContent = ….
Guard or fail loudly:-const output = document.getElementById("convert-output"); +const output = document.getElementById("convert-output"); +if (!output) { + console.error("convert-output element not found"); + return; +}(But preferred fix is adding the element in HTML, see earlier comment.)
🧹 Nitpick comments (1)
.gitignore (1)
19-20: Duplicate.qodopattern – keep only one entry
.qodo/is already ignored on line 19, yet a second (slightly different) entry.qodois appended at line 139.
Keeping both:
- clutters the ignore-list,
- might confuse future contributors about the canonical spelling (with or without trailing slash).
-CHANGELOG.* -.qodo +CHANGELOG.*If the directory form (
.qodo/) is correct, the extra line should be removed; if individual files inside.qodoalso need ignoring, keep the former and use/.qodo*(or similar) instead.Also applies to: 138-139
🛑 Comments failed to post (3)
index.css (1)
345-435: 🛠️ Refactor suggestion
Large block of duplicated rules inflates bundle & invites drift
Selectors such as
.tree-children,.tree-key,body.dark-mode .tree-key[title]:hover::after, etc. are repeated almost verbatim (first block lines 345-435, second block 491-575).Besides ~230 extra lines, duplication causes:
• heavier CSS payload,
• maintenance headaches (one copy updated, the other forgotten),
• potential specificity battles.Please keep one canonical definition and delete the clone.
-.tree-children { - /* second duplicate block… */ -} -… - -.tree-key.shift-toggle { - background-color: rgba(0,123,255,.08); -} +/* duplicate block removed – original rules above remain */Also applies to: 491-575
🤖 Prompt for AI Agents (early access)
In index.css between lines 345-435 and 491-575, there is a large block of duplicated CSS rules for selectors like .tree-children, .tree-key, and body.dark-mode .tree-key[title]:hover::after. This duplication increases the CSS bundle size and risks maintenance issues. To fix this, remove the duplicated block from lines 491-575 and keep only one canonical definition of these styles in lines 345-435, ensuring all necessary styles remain intact in the single retained block.index.html (1)
110-116:
⚠️ Potential issue
convert()will fail – missingid="convert-output"
index.jsexpectsdocument.getElementById("convert-output")(see lines 1073-1076) but the<pre>added here lacks that id.
convert()now dereferencesnull, throwingTypeError: Cannot set property 'textContent' of null.-<pre class="code-output" style="margin-top: 10px; overflow: auto"></pre> +<pre id="convert-output" + class="code-output" + style="margin-top:10px;overflow:auto"></pre>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<textarea id="convert-input" class="json-input" placeholder="Paste here..."></textarea> <button onclick="convert()">Convert</button> <button class="copy-button" onclick="copyConvertOutput()"> Copy Output </button> <pre id="convert-output" class="code-output" style="margin-top:10px;overflow:auto"></pre> </div>🤖 Prompt for AI Agents (early access)
In index.html around lines 110 to 116, the <pre> element used for output is missing the id "convert-output" which is required by the convert() function in index.js. Add id="convert-output" to the <pre> tag so that document.getElementById("convert-output") correctly references this element and prevents the TypeError when setting textContent.index.js (1)
1493-1523: 🛠️ Refactor suggestion
Function redeclared — pick one implementation
openTabRenameTooltipis defined earlier (lines 909-944) for JSON tabs (signature(tabId, mode)) and re-defined here for editor tabs (signature(tabId)), silently overwriting the first.
Side-effects:• Formatter/Compare/Codegen now call the new version with two args – ignored but confusing.
• Static analysis already warns (noRedeclare).Rename the editor-specific one (e.g.,
openEditorTabRenameTooltip) or merge both behaviours into a single, generic implementation.-function openTabRenameTooltip(tabId) { +function openEditorTabRenameTooltip(tabId) { @@ }Update calls in
addEditorTab()and elsewhere accordingly.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function openEditorTabRenameTooltip(tabId) { const btn = document.querySelector(`#editor-tabs-container .tab-button[data-tab="${tabId}"]`); const existing = document.querySelector(".tab-rename-tooltip"); if (existing) existing.remove(); const tooltip = document.createElement("div"); tooltip.className = "tab-rename-tooltip"; const rect = btn.getBoundingClientRect(); tooltip.style.left = rect.left + "px"; tooltip.style.top = rect.bottom + window.scrollY + 5 + "px"; const input = document.createElement("input"); input.type = "text"; input.value = btn.querySelector(".tab-name").textContent; input.style.width = "150px"; input.addEventListener("keydown", (e) => { if (e.key === "Enter") finalizeRename(); else if (e.key === "Escape") tooltip.remove(); }); input.addEventListener("blur", finalizeRename); tooltip.appendChild(input); document.body.appendChild(tooltip); input.focus(); function finalizeRename() { const newName = input.value.trim(); if (newName) { btn.querySelector(".tab-name").textContent = newName; } tooltip.remove(); updateEditorGlobalState(); } }🧰 Tools
🪛 Biome (1.9.4)
[error] 1493-1494: Shouldn't redeclare 'openTabRenameTooltip'. Consider to delete it or rename it.
'openTabRenameTooltip' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents (early access)
In index.js around lines 1493 to 1523, the function openTabRenameTooltip is redeclared, overwriting the earlier version defined around lines 909 to 944, causing confusion and static analysis warnings. Rename this second function to openEditorTabRenameTooltip to distinguish it from the JSON tabs version, and update all calls to this function in addEditorTab() and other places to use the new name. This avoids silent overwrites and clarifies which function is used for editor tabs.
Summary by CodeRabbit
New Features
Improvements