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.

Sign up for GitHub

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

Jump to bottom

feat: Add "Enable all rules" option in playground #462

Merged

nzakas merged 5 commits into eslint:main from amareshsm:enable-all-rules

Jan 29, 2024

Member

amareshsm commented Jul 30, 2023 •

edited

Loading

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request?

What changes did you make? (Give an overview)

Added "Enable all rules" option to enable all the rules in one go.

Screen.Recording.2024-01-25.at.1.23.00.AM.mov

Related Issues

Is there anything you'd like reviewers to focus on?


          feat: Add "Enable all rules" option in playground

f7af8c1

eslint-github-bot bot added the feature label

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for new-eslint ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/new-eslint/deploys/65b16967ca55d70008a12b4f
😎 Deploy Preview	https://deploy-preview-462--new-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for ja-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/ja-eslint/deploys/65b16967d10d730008d1e9b0
😎 Deploy Preview	https://deploy-preview-462--ja-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for es-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/es-eslint/deploys/65b169673b9e180008c753d5
😎 Deploy Preview	https://deploy-preview-462--es-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for hi-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/hi-eslint/deploys/65b16967dbfd0f0008e61d6e
😎 Deploy Preview	https://deploy-preview-462--hi-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for fr-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/fr-eslint/deploys/65b16967d933ab0008e51e62
😎 Deploy Preview	https://deploy-preview-462--fr-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for pt-br-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/pt-br-eslint/deploys/65b169671a0fa8000843c8c7
😎 Deploy Preview	https://deploy-preview-462--pt-br-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for de-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/de-eslint/deploys/65b16967a3025b00085d168f
😎 Deploy Preview	https://deploy-preview-462--de-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jul 30, 2023 •

edited

Loading

✅ Deploy Preview for zh-hans-eslint ready!

Name	Link
🔨 Latest commit	`1e9d4a7`
🔍 Latest deploy log	https://app.netlify.com/sites/zh-hans-eslint/deploys/65b169672e5e4f00085049c2
😎 Deploy Preview	https://deploy-preview-462--zh-hans-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Member

nzakas commented Aug 2, 2023

I'm not sure the UI is right on this. If I check the checkbox then it adds all rules, but when I uncheck the box, I'm not sure what it's supposed to do.

Member Author

amareshsm commented Aug 2, 2023 •

edited

Loading

I'm not sure the UI is right on this. If I check the checkbox then it adds all rules, but when I uncheck the box, I'm not sure what it's supposed to do.

As discussed Allow the selection of "all rules" in the playground #423 (comment) we can give a reset (Rest to default) option to restore the ESLint recommend rules.
When the "Enable all the rules" is unchecked maybe we can restore the previously enabled rule. let me know your thoughts on this.

Member

nzakas commented Aug 14, 2023

I don't think a checkbox is the right way to implement this. Maybe we should use a button instead? So it says "Enable All Rules" and when you click on it, it changes to "Reset Rules" (or something similar)?

Member Author

amareshsm commented Sep 8, 2023

I don't think a checkbox is the right way to implement this. Maybe we should use a button instead? So it says "Enable All Rules" and when you click on it, it changes to "Reset Rules" (or something similar)?

Yeah, we can have a button and change it accordingly. I will make the changes.


          chore: add button instead of checkbox

b755a8e

harish-sethuraman marked this pull request as ready for review

December 17, 2023 16:50

Member

harish-sethuraman commented Dec 17, 2023

changes made by me:

changed the checkbox to be a button
The select will also disappear when all the rules are selected. I did this because the select wont have anything when all rules are selected.

Member

harish-sethuraman commented Dec 21, 2023

@nzakas can you take a look at this now? @eslint/website-team would like some review on this one?

kecrily reviewed

View reviewed changes

src/assets/scss/components/playground-configuration.scss Outdated Show resolved Hide resolved


          chore: remove un used scss

78b918c

Member Author

amareshsm commented Dec 23, 2023

LGTM, I have updated the PR description with the latest changes. Thanks @harish-sethuraman

amareshsm added the accepted label

amareshsm linked an issue

that may be closed by this pull request

Allow the selection of "all rules" in the playground #423

Closed

1 task

Member

nzakas commented Dec 28, 2023

@harish-sethuraman I'm still seeing the select even when all rules are enabled.

I also feel that the button is just way too big. I don't think this is a common use case, so it shouldn't be reflected with such a large UI element. Instead, I'd suggest adding a link next to "Add Rule" that says "Add All Rules".

Member

harish-sethuraman commented Dec 29, 2023 •

edited

Loading

@nzakas probably all the rules are not selected in the screenshot you shared? Can you try opening the dropdown to check if it is empty? because this is how it looks for me when all rules are selected.

Screenshot 2023-12-29 at 12 58 56 PM

I'd suggest adding a link next to "Add Rule" that says "Add All Rules".

~~makes sense will change~~. There seems to be a problem with it, the disable all rules button will then move to the bottom of the list and disappear because the list is too large

Member

nzakas commented Jan 2, 2024

I just checked again. When I click the button, I get errors in the console and nothing happens. I'm using Firefox.

I still think the button is too big regardless of the functionality.


          chore: fix the layouting and firefox issue

9b0093f

Member

harish-sethuraman commented Jan 13, 2024 •

edited

Loading

@nzakas Have fixed the above issue. and changed the button to a smaller one similar to add rule button 👍🏻

https://deploy-preview-462--new-eslint.netlify.app/play/

Member

nzakas commented Jan 17, 2024

Thanks! This is looking good. Only issue now: When I click "Enable all rules", it sets focus to the most recently added textbox. That works okay when there are no rules enabled, but if you already have a bunch of rules enabled, then it scrolls the entire side panel down. Can we change it so that it always sets focus to the first visible textbox instead?

Member

harish-sethuraman commented Jan 20, 2024 •

edited

Loading

there are two cases

When we add rules separately, we focus only on the recently added one (alphabetical order).
- when we select rules while other rules exists we focus on recent one (alphabetical order) only. Even if the recently added one (alphabetical order) is pretty far away we scroll and focus
when we enable all rules it adds all rules and then focuses on the first one
- when we already have some rules enabled and enable all rules again it focuses on the recently added one (alphabetical order)

They both seem to be consistent.

Member

nzakas commented Jan 22, 2024

when we enable all rules it adds all rules and then focuses on the first one
* when we already have some rules enabled and enable all rules again it focuses on the recently added one (alphabetical order)

Yes, I'm saying this behavior is not desirable because it can cause the sidebar to scroll, which is very disorienting.


          make focus consistent

1e9d4a7

Member Author

amareshsm commented Jan 24, 2024

when we enable all rules it adds all rules and then focuses on the first one
* when we already have some rules enabled and enable all rules again it focuses on the recently added one (alphabetical order)
Yes, I'm saying this behavior is not desirable because it can cause the sidebar to scroll, which is very disorienting.

I have updated the PR accordingly now it always sets focus to the first visible textbox.

harish-sethuraman requested a review from nzakas

January 29, 2024 08:32

nzakas approved these changes

View reviewed changes

Member

nzakas left a comment

Perfect! Thanks for all the work on this.

nzakas merged commit 44cfc74 into eslint:main

37 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted feature