Skip to content

Various fixes [read below]#5

Open
fterrier wants to merge 27 commits into
arosequist:masterfrom
fterrier:master
Open

Various fixes [read below]#5
fterrier wants to merge 27 commits into
arosequist:masterfrom
fterrier:master

Conversation

@fterrier

@fterrier fterrier commented Jan 20, 2015

Copy link
Copy Markdown

Hi @arosequist,
Thanks for the great library, I use it and it seems to work quite well.

Here's a pull request with a few proposals on things that can be improved:

  • I fixed a problem where the "Loading" was only displayed for a flicker and then gone, it now works properly
  • Also fixed an issue with an empty
      being visible at some point, it was pushing the content down a few pixels so I've set that to "display: none;"
    • I've refactored the way the channels are created and killed in order to fix some potential race conditions. I'm not sure about that one perhaps you want to have a good second look?
    • I've added a IWilUnmount that closes all the channels
    • I've merged @robili's branch as well

    Let me know what you think,
    Franz


    This change is Reviewable

@fterrier

fterrier commented Feb 6, 2015

Copy link
Copy Markdown
Author

Hi @arosequist, @swannodette,

I've added a few commits to the PR and changed the module quite a lot:

  • I have fixed the issue with the dropdown disappearing before a selection could be made
  • I have made the module a little less customizable but a little easier to use, by providing sensible defaults and a "bootstrap wrapper" that just adds the correct classes to the elements. I have fixed your example so they still work and they need a little less code to work so I guess that's a step in the right direction? Let me know what you think.
  • I have also added a few "features" that I needed, such as the ability to provide your onKeyDown handler and a input-focus-ch that is used to set the focus on the input field from the outside (that one does not really work well on iPads and other tablets yet.)

Franz

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