feat: add "Apply Fix" and "Apply Suggestion" buttons with html-eslint #361
Conversation
HyzeahAchungha
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds "Apply Fix" and "Apply Suggestion" buttons to the code editor interface, integrating html-eslint for automated linting and code suggestions.
- Implements event listeners for applying fixes and suggestions
- Introduces helper methods (getLintConfig and applyFix) for handling lint configuration and applying fixes
- Adjusts the layout and HTML to load the updated app script
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/website/src/scripts/playground/app.js | Implements new linting functionality and buttons for applying fixes/suggestions |
| packages/website/src/scripts/layout.js | Minor formatting update |
| packages/website/src/index.html | Adds script tag to load the updated playground app |
| this.handleCodeChange(this.view.codeEditor); | ||
| this.handleConfigChange(this.view.configEditor); | ||
| this.handleLanguageChange(); | ||
|
|
||
| // Setup fix/suggestion buttons | ||
| const applyFixBtn = document.getElementById("applyFixBtn"); |
There was a problem hiding this comment.
Consider adding null checks for 'applyFixBtn', 'applySuggestionBtn', and 'lintOutput' elements to handle cases where these elements might not exist, preventing potential runtime errors.
Copilot uses AI. Check for mistakes.
|
|
||
| applySuggestionBtn.addEventListener("click", () => { | ||
| const code = this.view.codeEditor.getValue(); | ||
| const eslintConfig = this.getLintConfig(); |
There was a problem hiding this comment.
Silently returning an empty config in getLintConfig() may hide invalid JSON configurations; consider logging the error or notifying the user to improve debuggability.
Copilot uses AI. Check for mistakes.
| const newCode = this.applyFix(code, suggestion.fix); | ||
| this.view.codeEditor.setValue(newCode); | ||
| lintOutput.textContent = `✅ Applied suggestion: ${suggestion.desc}`; |
There was a problem hiding this comment.
Ensure that 'suggestion.fix' is defined before applying the fix, as some suggestions might not include an auto-fix, which could lead to runtime errors.
| const newCode = this.applyFix(code, suggestion.fix); | |
| this.view.codeEditor.setValue(newCode); | |
| lintOutput.textContent = `✅ Applied suggestion: ${suggestion.desc}`; | |
| if (suggestion.fix) { | |
| const newCode = this.applyFix(code, suggestion.fix); | |
| this.view.codeEditor.setValue(newCode); | |
| lintOutput.textContent = `✅ Applied suggestion: ${suggestion.desc}`; | |
| } else { | |
| lintOutput.textContent = "ℹ️ No fix available for the selected suggestion."; | |
| } |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@HyzeahAchungha Thank you for your contribution! I've left a review.
There was a problem hiding this comment.
Hi @HyzeahAchungha This project does not use package-lock.json. Could you please remove this file?
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Code Playground</title> | ||
| <script src="https://cdn.tailwindcss.com"></script> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/codemirror.min.css"> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/codemirror.min.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/mode/htmlmixed/htmlmixed.min.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/mode/javascript/javascript.min.js"></script> | ||
| <!-- ESLint for real linting and auto-fix --> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/eslint/8.56.0/eslint.min.js"></script> |
There was a problem hiding this comment.
Since layout.html is used as a template via posthtml, the html & head & body tag is unnecessary. Also, codemirror is already specified in package.json, so there's no need to include it via CDN. Could you please remove these parts?
| } | ||
| if ($openTarget || $closeTarget) { | ||
|
|
||
| <script type="module"> |
There was a problem hiding this comment.
Could you move the JavaScript code to packages/website/src/scripts/playground/app.js and remove the inline script?
There was a problem hiding this comment.
There were no changes to the dependencies, so there shouldn’t be any modifications to the yarn.lock file. Could you please exclude it from the commit?
|
Okay I will remove it
…On Sun, Jun 1, 2025 at 2:52 PM YeonJuan ***@***.***> wrote:
***@***.**** requested changes on this pull request.
@HyzeahAchungha <https://github.com/HyzeahAchungha> Thank you for your
contribution! I've left a review.
------------------------------
On package-lock.json
<#361 (comment)>:
Hi @HyzeahAchungha <https://github.com/HyzeahAchungha> This project does
not use package-lock.json. Could you please remove this file?
------------------------------
In packages/website/src/components/playground.html
<#361 (comment)>:
> +<!DOCTYPE html>
+<html lang="en">
+ <head>
+ <meta charset="UTF-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1.0">
+ <title>Code Playground</title>
+ <script src="https://cdn.tailwindcss.com"></script>
+ <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/codemirror.min.css">
+ <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/codemirror.min.js"></script>
+ <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/mode/htmlmixed/htmlmixed.min.js"></script>
+ <script src="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.2/mode/javascript/javascript.min.js"></script>
+ <!-- ESLint for real linting and auto-fix -->
+ <script src="https://cdnjs.cloudflare.com/ajax/libs/eslint/8.56.0/eslint.min.js"></script>
Since layout.html is used as a template via posthtml, the html & head &
body tag is unnecessary. Also, codemirror is already specified in
package.json, so there's no need to include it via CDN. Could you please
remove these parts?
------------------------------
In packages/website/src/components/playground.html
<#361 (comment)>:
> - const $fixed = document.getElementById("fixed");
-
- if (e.target) {
- let $openTarget = null;
- let $closeTarget = null;
-
- if (e.target.dataset.tab === "errors") {
- $openTarget = $errors;
- $closeTarget = $fixed;
- } else if (e.target.dataset.tab === "fixed") {
- $openTarget = $fixed;
- $closeTarget = $errors;
- }
- if ($openTarget || $closeTarget) {
+
+ <script type="module">
Could you move the JavaScript code to
packages/website/src/scripts/playground/app.js and remove the inline
script?
------------------------------
On yarn.lock
<#361 (comment)>:
There were no changes to the dependencies, so there shouldn’t be any
modifications to the yarn.lock file. Could you please exclude it from the
commit?
—
Reply to this email directly, view it on GitHub
<#361 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A2B2UCPJBMXVCJ7C7SAH7BL3BMASPAVCNFSM6AAAAAB6IYRY56VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQOBVGY2DIOBSGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
FEATURE Add Apply Fix and Apply Suggestion
Added event listeners for #applyFixBtn and #applySuggestionBtn
Used Linter.verifyAndFix() and Linter.verify() from html-eslint for linting and suggesting
Updated the code editor with the modified code when fixes/suggestions are applied
Displayed success or info messages in #lintOutput
This PR implements the "Apply Fix" and "Apply Suggestion" buttons in the application interface. It integrates the html-eslint parser and linter to:
Automatically apply ESLint auto-fixes to the user’s code
Allow users to apply suggested fixes manually if available
Display feedback messages to inform users whether fixes or suggestions were applied