chore: add apply suggestion, fix on playground #397
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds autofix functionality to the playground by implementing apply fix and apply suggestion buttons for ESLint messages. The changes enable users to automatically apply ESLint fixes and suggestions directly from the error display.
- Added UI buttons for applying fixes and suggestions to lint messages
- Implemented backend logic to apply fixes and suggestions by modifying the code editor content
- Enhanced the error display with better styling and overflow handling
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/website/src/scripts/playground/view.js | Added error element caching and redesigned lint message HTML with fix/suggestion buttons |
| packages/website/src/scripts/playground/model.js | Implemented applyFix and applySuggestion methods with autofixed event handling |
| packages/website/src/scripts/playground/app.js | Added event listeners for autofix buttons and autofixed event handling |
| packages/website/src/components/playground.html | Added overflow scroll to error container and removed placeholder text |
| <a href="/docs/rules/${rule}" class="ml-1 hover:underline">(${rule})</a> | ||
| </span> | ||
| <div class="ml-auto mr-0 my-2"> | ||
| ${fix? html`<button class="bg-accent text-white px-4 py-1 rounded hover:opacity-80 w-max">apply fix</button>`: ""} |
There was a problem hiding this comment.
Missing space after the ternary operator condition. Should be ${fix ? html... : ""}
| code.slice(end); | ||
| this.setCode(fixed); | ||
| this.lint(); | ||
| this.notify('autofixed') |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. JavaScript best practices recommend consistent semicolon usage.
| this.notify('autofixed') | |
| this.notify('autofixed'); |
| const fixed = code.slice(0, start) + | ||
| suggestion.fix.text + | ||
| code.slice(end); | ||
| this.setCode(fixed); |
There was a problem hiding this comment.
Trailing whitespace at the end of the line should be removed.
| const $button = event.target.closest("button"); | ||
| if ($button) { | ||
| const $li = $button.closest("li"); | ||
| const index = Array.from($li.parentNode.children).indexOf($li); |
There was a problem hiding this comment.
This approach to find the message index is fragile and could break if the DOM structure changes. Consider storing the message index as a data attribute on the list item instead.
| const index = Array.from($li.parentNode.children).indexOf($li); | |
| const index = $li.dataset.index ? parseInt($li.dataset.index, 10) : -1; |
| const $li = $button.closest("li"); | ||
| const index = Array.from($li.parentNode.children).indexOf($li); | ||
| const message = this.model.messages[index]; | ||
| if ($button.textContent.includes("fix")) { | ||
| this.model.applyFix(message); | ||
| } else if ($button.textContent.includes("suggestion")) { |
There was a problem hiding this comment.
Using textContent to determine button action is brittle and could break with internationalization or text changes. Consider using data attributes or CSS classes to identify button types.
| const $li = $button.closest("li"); | |
| const index = Array.from($li.parentNode.children).indexOf($li); | |
| const message = this.model.messages[index]; | |
| if ($button.textContent.includes("fix")) { | |
| this.model.applyFix(message); | |
| } else if ($button.textContent.includes("suggestion")) { | |
| // NOTE: Buttons must have a data-action attribute set to "fix" or "suggestion" | |
| const $li = $button.closest("li"); | |
| const index = Array.from($li.parentNode.children).indexOf($li); | |
| const message = this.model.messages[index]; | |
| if ($button.dataset.action === "fix") { | |
| this.model.applyFix(message); | |
| } else if ($button.dataset.action === "suggestion") { |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #397 +/- ##
=======================================
Coverage 98.53% 98.53%
=======================================
Files 82 82
Lines 2658 2658
Branches 736 736
=======================================
Hits 2619 2619
Misses 39 39
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Checklist
Description