Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged

Conversation

Josh-Cena
Copy link
Member

@Josh-CenaJosh-Cena commentedMay 31, 2022
edited
Loading

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.

@nx-cloud
Copy link

nx-cloudbot commentedMay 31, 2022
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commitfc3eadf. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 47 targets

Sent with 💌 fromNxCloud.

@typescript-eslint
Copy link
Contributor

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.

@netlify
Copy link

netlifybot commentedMay 31, 2022
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commitfc3eadf
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/62b7583cbb3c2b0007d668bf
😎 Deploy Previewhttps://deploy-preview-5116--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site settings.

@Josh-Cena
Copy link
MemberAuthor

Josh-Cena commentedMay 31, 2022
edited
Loading

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.

@armano2
Copy link
Collaborator

Do we want to use the same card view instead of table view?

maybe something like this?

image

@Josh-Cena
Copy link
MemberAuthor

Let's see—much easier to experiment now.

@Josh-Cena
Copy link
MemberAuthor

Josh-Cena commentedMay 31, 2022
edited
Loading

OK the filter logic is hard to make... We have three possible states:

  • include: if a rule has attribute X, immediately include it in the list
  • exclude: if a rule has attribute X, immediately exclude it from the list
  • neutral: hold a neutral option and yield to other filters to decide whether the current rule should be included

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.

@bradzacherbradzacher added the documentationDocumentation ("docs") that needs adding/updating labelJun 1, 2022
Copy link
Member

@bradzacherbradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this is looking good to me!

damngoodstuff

bradzacher
bradzacher previously requested changesJun 1, 2022
Copy link
Member

@bradzacherbradzacher left a 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.

@armano2
Copy link
Collaborator

what do you guys think about changing those tables to have css

    display: table;    width: 100%;

as right now while filtering size of it can shift alot

@Josh-Cena
Copy link
MemberAuthor

Josh-Cena commentedJun 1, 2022
edited
Loading

display: table;width:100%;

Do I look like I know any CSS... 😄

I'll admit I tried this, and setwidth: 100% fortable,thead, and everything else—but the table still shifts when there are no subitems.

@Josh-CenaJosh-Cenaforce-pushed thefilterable-table branch 2 times, most recently from803ff11 toa8c8567CompareJune 1, 2022 05:12
@armano2
Copy link
Collaborator

4.5em looks a bit too wide for me... Not sure if that's necessary for other devices though.

that's depends if we want to keep them vertically or horizontally


if horizontally I think its actually betterwhite-space: nowrap; instead of defining width

@Josh-Cena
Copy link
MemberAuthor

Pushed a layout with vertically arranged icons. I like it better

armano2 reacted with thumbs up emoji

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a 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 💖!

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJun 2, 2022
@armano2
Copy link
Collaborator

related issue#4366

@armano2
Copy link
Collaborator

armano2 commentedJun 10, 2022
edited
Loading

wdyt about changing filters a little

image

this is just and idea, i'm not sure if good one 🐱

@Josh-Cena
Copy link
MemberAuthor

"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
Copy link
Member

JoshuaKGoldberg commentedJun 11, 2022
edited
Loading

good idea to not show "recommended" rules as not included in "strict"

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.

@Josh-Cena
Copy link
MemberAuthor

A rule can only be in one of those two rulesets. Rules that are in strict are not in recommended, and vice versa.

Ugh, I have made that realization at least twice during my course of working on this, but I keep forgetting... Thanks for reminding again...

JoshuaKGoldberg reacted with laugh emoji

@bradzacherbradzacher added awaiting responseIssues waiting for a reply from the OP or another party and removed awaiting responseIssues waiting for a reply from the OP or another party labelsJun 13, 2022
@JoshuaKGoldberg
Copy link
Member

Re ☝️ ,#5204

@JoshuaKGoldberg
Copy link
Member

@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.

@Josh-Cena
Copy link
MemberAuthor

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 reacted with heart emoji

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commentedJun 25, 2022
edited
Loading

High schoolers doing a hands together and shouting "teamwork!"

Edit: and, welcome to the states! 🙌

@JoshuaKGoldbergJoshuaKGoldberg dismissed stale reviews frombradzacher and themselfJune 25, 2022 18:48

Outdated

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a 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!:shipit:

@JoshuaKGoldbergJoshuaKGoldberg merged commit507629a intotypescript-eslint:mainJun 25, 2022
@Josh-CenaJosh-Cena deleted the filterable-table branchJune 26, 2022 02:03
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsAug 6, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@bradzacherbradzacherbradzacher left review comments

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
awaiting responseIssues waiting for a reply from the OP or another partydocumentationDocumentation ("docs") that needs adding/updating
Projects
None yet
Milestone
No milestone
4 participants
@Josh-Cena@armano2@JoshuaKGoldberg@bradzacher

[8]ページ先頭

©2009-2025 Movatter.jp