jquery / sizzle

A sizzlin' hot selector engine.
https://sizzlejs.com
Other
6.29k stars 951 forks source link

Selector: Add :invalid and :valid selectors #413

Closed FooBarrior closed 2 years ago

FooBarrior commented 6 years ago

Add :invalid and :valid pseudoclass selectors.

The implementation is based on element's validity attribute.

If element has no validity attribute, it's supposed, that both functions return false. This also matches CSS behaviour.

jsf-clabot commented 6 years ago

CLA assistant check
All committers have signed the CLA.

mgol commented 6 years ago

What do you think, @gibson042?

timmywil commented 6 years ago

We've rejected this in the past because there are jQuery validation plugins that add these selectors.

linux-foundation-easycla[bot] commented 2 years ago

CLA Signed

The committers are authorized under a signed CLA.

FooBarrior commented 2 years ago

Hello @mgol! Are you going to accept this change? I can see no resolution about it here. If yes, should I work on fixing lint error?

It seems that error makes no sense: the line number is wrong, and there are more than one comment dissatisfying lint warning in the file

mgol commented 2 years ago

@FooBarrior the reference doesn't seem to make sense as GitHub first merges the PR to the latest main & only then runs the build. The main branch diverged enough that the line numbers are now different than what you can see on your branch. If you rebase over latest main or merge main to itself you should see the expected line numbers.

As for the code itself, Richard's remark would have to be addressed before we can consider merging the change. Also, at this point of the project lifecycle we are not likely to accept big changes and I don't see how we can address Richard's remark without adding lots of code... I don't see a way out at the moment, unfortunately.

FooBarrior commented 2 years ago

Ok, you can just close this PR again. I only asked because I saw there were some apparent movements there.

On Tue, 11 Jan 2022 at 01:21, Michał Gołębiowski-Owczarek < @.***> wrote:

@FooBarrior https://github.com/FooBarrior the reference doesn't seem to make sense as GitHub first merges the PR to the latest main & only then runs the build. The main branch diverged enough that the line numbers are now different than what you can see on your branch. If you rebase over latest main or merge main to itself you should see the expected line numbers.

As for the code itself, Richard's remark would have to be addressed before we can consider merging the change. Also, at this point of the project lifecycle we are not likely to accept big changes and I don't see how we can address Richard's remark without adding lots of code... I don't see a way out at the moment, unfortunately.

— Reply to this email directly, view it on GitHub https://github.com/jquery/sizzle/pull/413#issuecomment-1009400368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADAICJFW6HMD4C7ZESZZDTUVNLV3ANCNFSM4ELMWYEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- Yours truly, Nikita Malyavin

mgol commented 2 years ago

Thanks for the update; closing then.