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

Support Chrome autofill #2395

Closed
wants to merge 10 commits into from
Closed

Conversation

davidcoleman007
Copy link

This PR addresses #1397 .

Originally a PR was raised by @sevenval and @mmintel. that pr is: #2174

The purpose of this PR is to resolve the merge conflicts present in the original PR, and provide a working proposal for merge with current master as of 2/22/18.

My work is available at: https://github.com/davidcoleman007/react-select

mmintel and others added 10 commits January 11, 2018 13:13
looks like I missed this one line after reviewing my diff

remove dist
restore old dist

revert this
Merge remote-tracking branch 'origin/master'
looks like I missed this one line after reviewing my diff

remove dist
restore old dist

revert this
Merge remote-tracking branch 'origin/master'
Merge branch 'master' of https://github.com/davidcoleman007/react-select

restore dist
@davidcoleman007
Copy link
Author

I noticed that in the original PR, it was asked that dist not be touched, so I've restored the dist from master to this pr, so that none of my work is overwriting dist files. Please let me know if there are further considerations which I must take.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.8%) to 93.126% when pulling 1239ba8 on davidcoleman007:master into c9d0273 on JedWatson:master.

@jossmac
Copy link
Collaborator

jossmac commented Mar 9, 2018

@JedWatson needs review. Happy to pair on this tomorrow morning if you have the time.

@davidcoleman007
Copy link
Author

@jossmac Thank you! Let me know how I can help!

@fluke
Copy link

fluke commented Apr 16, 2018

@jossmac Will this be merged for the v2 or after?

@matheusazzi
Copy link

Any news on that? @davidcoleman007 @JedWatson

@spencern
Copy link

spencern commented Oct 2, 2018

Any chance this PR gets a review soon?

@davidcoleman007
Copy link
Author

i can focus on resolving the conflicts if someone can commit to actually reviewing it. I've kinda given up on this work since it's been ignored by the maintainers for so long.

@davidcoleman007
Copy link
Author

Many people need this feature. This project is in wide use in the wild and I propose that those of us who are active and interested in maintaining this repo should be free to do so.

@bhargavjoshi
Copy link

Any updates on this? I would love this feature get released.

@davidcoleman007
Copy link
Author

I intend to put time in on this over the weekend. I'll get these conflicts put to bed and we can roll this out now that I've got approvals!

@spencern
Copy link

Thanks for your work on this @davidcoleman007

@davidcoleman007
Copy link
Author

davidcoleman007 commented Oct 21, 2018

The conversion to css-in-js from scss and less has caused this to be significantly greater than a conflicts resolution. I'm working on re-implementing the css changes over the latest HEAD. This PR is no longer compatible with HEAD. I'm working towards fixing it.

@nisargrthakkar
Copy link

Is there any update on chrome autofill?

@krnlde
Copy link

krnlde commented Mar 13, 2020

I'd like to bump this PR kindly :)

@bladey
Copy link
Contributor

bladey commented Jun 3, 2020

Hi @davidcoleman007,

Thanks for your efforts here.

Regarding your reply here - #2395 (comment)

If this PR is still relevant and is updated, I'd gladly get this reviewed and merged for you.

@bladey bladey added awaiting-author-response Issues or PRs waiting for more information from the author enhancement and removed deprecated labels Jun 3, 2020
@bladey bladey linked an issue Jun 12, 2020 that may be closed by this pull request
@bladey bladey added pr/enhancement PRs that are specifically enhancing the project and removed category/enhancement labels Jun 24, 2020
@ebonow
Copy link
Collaborator

ebonow commented Jan 16, 2021

Greetings @davidcoleman007

It seems like a lot of time was put into this PR and seems to have a lot of interest from users. I wanted to thank you for the effort and time you put into making this for the community. It seems unacceptable that so much time had passed from your time of development to a response about having it reviewed. I apologize that this was the case and our contributor community deserves to know that they are heard and trying to contribute to a mutual cause.

As it stands, there is quite a backlog that I am committed to responding to so that those taking time out of their lives to contribute receive some kind of response. Since the changes were made against v1 it would require quite a bit of re-work to get it up-to-date.

For me personally, since the behavior seems to only support webkit browsers, I also have concerns about implementing and supporting a feature that is not browser agnostic. As it stands, I can see this being developed as an add-on or react-hook as I think the underlying functionality would be helpful to many. I will take on the case of trying to implement this without touching source code so that users can roll their own implementation.

Should this meet the criteria of both a v3+ rewrite and support for at least Mozilla as well, I would happily have this reviewed and recommended. I will be closing this for now, but will update with a comment when I am able to compose a working codesandbox. Please keep in mind that this and can be reopened should the above criteria be met and the team and community appreciate any and all efforts to improve react-select.

@davidcoleman007
Copy link
Author

@ebonow Thank you for that acknowledgement. I'll be frank and say that I'm so far out of the loop on this task that I am not able to rejoin the effort again. I also agree that this code is long in the tooth and should be done using any and all modern paradigms which may present themselves in the context of the new directions this repo will take. Had a timely response been received i surely would have iterated on this with the maintainers of the time, but today it's simply not something that i have the bandwidth for. I hope my work can illuminate someone else to take it the final mile.

@ebonow
Copy link
Collaborator

ebonow commented Feb 6, 2021

@davidcoleman007 I attempted to recreate this in a codesandbox without impacting the source code through custom components but then Chrome autofill refused to work even with recommended "hints" to trigger it (attributes for name, autocomplete, label, id, etc...). I will likely pick it up if there is traction for it, but again thank you for the contribution and hope it will serve as an inspiration for some future implementation.

@davidcoleman007
Copy link
Author

davidcoleman007 commented Feb 6, 2021

https://developers.google.com/web/updates/2015/06/checkout-faster-with-autofill

I remember that for sure. i also remember that you are depending on making the hidden input look good enough to the browser to get autofilled and then you have to handle that change and know that it means set autofill mode state in your react select's visual state

this can mean that you need to blast a previous selection, or that you need to hide the dropped down menu because if i remember correctly, the auto complete dropdown is over the dropped down list of react select items, and - yay dom events - it bubbles right through and selects whatever's under that menu, so when it enters that yellow state you gotta know it in the react comp and roll up the options.

however i don't think there's a dom event that says this happened, but there's a css animation that happens in chrome anyway, and THAT does have a dom event that we can check and see if it was for autofill... kinda hacky and not browser agnostic by a mile.

But hey it worked 2 or 3 years ago whenever that was.

Hope that all helps?

@ebonow
Copy link
Collaborator

ebonow commented Feb 6, 2021

I could tell a lot more consideration went into your PR outside of creating two animation keyframes and listening for webkitTransitionStart/End. It's worth noting that I did come across this while researching this that could possibly provide better browser support.

It's frustrating that vendor support for this is not standard and that it would be fine to ignore autocomplete "off" for the betterment of users, but at least provide a non-hacky implementation to detect these changes via javascript.

In any case, thanks for the blueprint and creating a way forward for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author-response Issues or PRs waiting for more information from the author pr/enhancement PRs that are specifically enhancing the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Support for Chrome autofill