Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
docs: autogenerate rules table on website#5116
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
docs: autogenerate rules table on website#5116
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nx-cloudbot commentedMay 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the PR,@Josh-Cena! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day. |
netlifybot commentedMay 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site settings. |
Uh oh!
There was an error while loading.Please reload this page.
Josh-Cena commentedMay 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I lazily reused a lot of styles fromhttps://docusaurus.io/showcase/. Do we want to use the same card view instead of table view? Also currently the filters look a bit raw. There are only two states: "include" and "don't care". Ideally each checkbox should be tri-state: "include", "don't care", and "exclude". But the UI/UX is a bit hard to design. |
Uh oh!
There was an error while loading.Please reload this page.
Let's see—much easier to experiment now. |
Josh-Cena commentedMay 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
OK the filter logic is hard to make... We have three possible states:
But if all filters are "neutral" about a rule (either because they have "neutral" state or because the rule has no attributes), should we include it? This means we either have to have an option "default to include/exclude", or we expand the states to much more states, because we have two cases: a rule with/without attribute X, each with three outcomes: immediately include, immediately exclude, and remain neutral. And after that comes the question of priority when one filter wants to include and the other wants to exclude. Filters are hard to build... The only alternative is to allow writing custom queries 😄 We may define priority based on the order of clicking, but that feels a bit unwieldy as well. Anyways, the UI is here; what UX we want is up for discussion. |
Uh oh!
There was an error while loading.Please reload this page.
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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
sorry... force of habit at meta where we stamp and let the author make changes and ship whenever.
what do you guys think about changing those tables to have css
as right now while filtering size of it can shift alot |
Josh-Cena commentedJun 1, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Do I look like I know any CSS... 😄 I'll admit I tried this, and set |
803ff11
toa8c8567
Compare
that's depends if we want to keep them vertically or horizontally if horizontally I think its actually better |
Pushed a layout with vertically arranged icons. I like it better |
Uh oh!
There was an error while loading.Please reload this page.
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.
Looks great, awesome stuff! Just requesting changes on the additional rule table removal as you suggested, and the accessibility touchups. Thanks@Josh-Cena 💖!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
related issue#4366 |
armano2 commentedJun 10, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
"fixable" / "has suggestions" are not necessarily mutually exclusive. In fact I'm not even sure if it's a good idea to not show "recommended" rules as not included in "strict" (sorry for the triple negation). I'm also thinking about a toggle between "AND" and "OR" of these filters, as seen on our showcase:https://docusaurus.io/showcase |
JoshuaKGoldberg commentedJun 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
A rule can only be in one of those two rulesets. Rules that are in strict are not in recommended, and vice versa. This is confusing and keeps coming up (why would you enable strict without recommended?) so maybe it'll change. But at least in version 5.X+ the two have no overlap. |
Ugh, I have made that realization at least twice during my course of working on this, but I keep forgetting... Thanks for reminding again... |
Re ☝️ ,#5204 |
@Josh-Cena just checking, is this ready for re-review? No worries if you haven't had time 🙂 just don't want to keep you waiting! This is going to be very good for the docs push, so I'm happy to take it over the finishing line if you're swamped with other things. |
I think the only pending issue is the accessibility of the badges, right? (Unless you guys want some other UX for the filters?) I'm indeed a bit occupied with other stuff these weeks (relocating to the US this summer so getting visas and all paperwork sorted out.) Would appreciate it if you can help out. Thanks a lot! |
JoshuaKGoldberg commentedJun 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Let's ship this thing!
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
Let me know if you think it's fine to remove the table in the package's README and simply point people to the website.