Skip to content
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

Making filtering text inputs easy / simple again #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pbodnar
Copy link

@pbodnar pbodnar commented May 1, 2017

This is de-facto a revert of the pull request "#54 from
abolishme/patch-1" (see the discussion there) and follow-up commits, while also clarifying how the
original code was possibly meant and bringing back some "reasonable defaults".

Plus fixing related (structure) typos in the README file.

This is de-facto a revert of the pull request "jeresig#54 from
abolishme/patch-1" and follow-up commits, while also clarifying how the
original code was meant and bringing back some "reasonable defaults".

Plus fixing related (structure) typos in the README file.
@pbodnar pbodnar mentioned this pull request May 1, 2017
@pbodnar
Copy link
Author

pbodnar commented May 1, 2017

Note that the I have left the filterContentEditable option untouched, so as firstly, not to remove all the recent work done and secondly, I am not quite sure if to filter even these types of elements by setting the seemingly universal-enough filterTextInputs option. Letting this decision to others for now...

Note 2: In the very original forked repository, one can see that these input elements were always filtered: select, textarea, input with apropriate type attribute and elements with the contenteditable attribute (and this behavior was not even switchable by an option).

@rpocklin
Copy link
Collaborator

rpocklin commented May 22, 2017

I think we need some tests here. The reason the option exists is because some users have asked for it (for better or worse).

@pbodnar
Copy link
Author

pbodnar commented Jul 10, 2021

OK, so is this project completely abandoned by the maintainers? The latest commits and documentation I tried to fix with this PR, together with wrong tagging (see 58038e6#diff-5736fc95626e37d54f44e395ad7aea8120507f84525485498a7d909db88a9e72) left this project in a very messy state, IMO. It looks like maintaining another fork is the only way to go now?

@rpocklin
Copy link
Collaborator

I'll be reviewing this and the previous PR over the next few days to consider this PR and possibly release a new version of the library.

@rpocklin
Copy link
Collaborator

@pbodnar can you please take a look at #63 which should do the minimum required to revert the behaviour whilst leaving the configuration customisable (and with your improvements to the docs)?

@pbodnar
Copy link
Author

pbodnar commented Nov 20, 2021

@rpocklin, thank you, I've left the feedback at the new PR. I hope you still have time? Anyway, I guess you can close this PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants