-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[AutoComplete] Migrate to use popover #2634
[AutoComplete] Migrate to use popover #2634
Conversation
@oliviertassinari @newoga This is more important that theme. we need this for 0.14.0 😁 Those can wait :P |
Sure, the theme question should really be answered but can wait a bit (it can have a lot of good impacts). I have noticed one change, the width of the Popover is different. |
check again please 😁 |
@subjectix Yes, that's better 👍. I haven't any other bugs. |
@oliviertassinari Thanks for reviewing this 😁 Can I merge after a squash? ^_^ |
08b79bf
to
f38b954
Compare
@subjectix @oliviertassinari I agree that the theme stuff can wait. Sorry for getting caught up in it. I haven't reviewed this (but I don't think I need to). The changes seem sound and I'm happy with it. 😄 |
@newoga Thanks ^.^ I myself have some issues with it, but they relate to how popover works. We should improve the API and possibly the behavior. It's a very great component, but it can be improved a lot. |
I agree! @yongxu's initial PR was the first time I contributed (via commenting) to this project. So I also have him to thank for getting me involved. 😄 |
I'm hoping when I get around #2416 I can also incorporate AutoComplete behavior from this. |
Yes, let's merge this. Sometimes we have to take risks and break stuff to move fast. |
[WIP] [AutoComplete] Migrate to use popover
@newoga We are glad you are here 👍 |
@oliviertassinari Thanks, sorry my code was a bit sloppy. We should fix a lot of stuff in 0.14.1 😁 |
Yeah you've been a great help so far 👍 👍 |
Thanks guys, I'm glad to be here! You both (and many others I've seen while perusing the issues as I often do in many projects) have been great examples of how to build an inviting community 👏 . |
@newoga Glad you like this community, I love it myself ❤️ ❤️ |
I'm still figuring out how the auto-complete works and why it uses instance properties instead of state.
Closes #2632 and #2595
@oliviertassinari Please check this out. It is still buggy, and sometimes forcefully holds the focus. I know it's coming from the callback passed to onBlur event of TextField, without it, it looks ugly. With it sometimes it won't leave the focus. play with the component for a while and you might be able to reproduce it (for me it's fairly easy)
@yongxu could use your help again, if you've got the time 😁