-
-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add disable rule button in playground messages #546
Conversation
✅ Deploy Preview for fr-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for es-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for zh-hans-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for hi-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for new-eslint ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@zhangenming pls create an issue explaining what problem you are trying to solve and the solution you would like to implement. ESlint team will triage the issue and mark it as accepted if the team agrees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it could be useful, but I'd like some other reviews from @eslint/eslint-team before merging.
@zhangenming Are you still working on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls resolve conflicts as well.
f176019
to
a84a3a8
Compare
a84a3a8
to
5cef364
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the preview and clicked on "disable" for the quotes rule. After clicking the disable button in the right panel, the rule was removed. However, despite clicking disable, the error is still displayed, and the disable option is still shown in the UI.
'm not sure how you are implementing the disable behavior. Could you please brief?
Screen.Recording.2024-07-31.at.12.24.22.AM.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM. Leaving it open in case anyone else wants to review before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Would like someone from @eslint/eslint-tsc to verify if this was intended behaviour.
observations:
- when a rule is selected through dropdown the disable button is shown
- when the button is clicked to disable then the rule is removed from list of rules
- inline enabling of lint rules doesn't pop disable button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Leaving open for @nzakas to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@harish-sethuraman confirmed. This is the intended behavior.
Prerequisites checklist
What is the purpose of this pull request?
we can Disabled an rule quickly.
What changes did you make? (Give an overview)
Related Issues
fixes #548
Is there anything you'd like reviewers to focus on?
The css style is a little newbie not perfect sorry