🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@DanielApt
Copy link

Closes #53

@jcrben
Copy link
Contributor

jcrben commented Jan 19, 2016

Looks good to me altho I didn't run it. Might want to update the docs too.

By the way, one way to make the maintainers perhaps more comfortable might be to demo the old and new behavior with a screencast http://www.screencast.com/

This seems straightforward enough tho...

@DanielApt
Copy link
Author

I've updated the typeahead docs here

I feel this feature is clear enough to not require a screencast, but happy to do so if you feel it's necessary.

@DanielApt
Copy link
Author

@jcrben It has been working as expected in production.

@synth
Copy link

synth commented Feb 27, 2016

+1 love to see this merged in

@synth
Copy link

synth commented Feb 27, 2016

actually, @DanielApt, I built this myself and caught a bug. The onblurred event doesn't have any arguments passed in and thus the referenced $e is undefined. Also there are situations when the event is called and the $e argument is undefined. I traced this back to input.js#74:

      _.mixin(Input.prototype, EventEmitter, {
            _onBlur: function onBlur() {
                this.resetInputValue();
                this.trigger("blurred");
            },

This event, and really all others, should pass along the event so it doesn't break standard js event model.

@jlbooker jlbooker added this to the v0.11.2 milestone Mar 2, 2016
@fedesoria
Copy link

$('.typeahead').bind('typeahead:render', function() {
  $('.typeahead').typeahead('moveCursor', +1);
});

@MarkHerhold
Copy link

@synth was the event passing issue introduced by this PR or was it a pre-existing 'feature' of the previous code-base?

If it is pre-existing and adding the event argument later will not cause a regression, I think this should be merged in. Or am I misunderstanding the situation?

@synth
Copy link

synth commented Apr 1, 2016

@MarkHerhold well, this code hasn't changed from twitter's codebase, so I'm pretty sure its always existed this way. 🚀 merge

@DanielApt
Copy link
Author

Sorry, been a bit out of the loop on this one. Would you want me to add @synth's code snippet to my PR?

@jlbooker jlbooker modified the milestones: v0.11.3, v0.11.2 Nov 2, 2016
@jlbooker
Copy link
Contributor

jlbooker commented Nov 2, 2016

Moving this to the 0.11.3 milestone, since it's not clear if this is ready to go.

@DanielApt and @synth is the onBlur event issue resolved?

@hutch120
Copy link

hutch120 commented May 9, 2017

Just migrated from the old twitter typeahead for other reasons, but our clients miss this feature. Looking forward to seeing it in the next release.

@JonathanSeguin
Copy link
Contributor

+1 when can we see this merged?

@jlbooker
Copy link
Contributor

@JonathanSeguin, When the merge conflicts are resolved. 😉

I likely won't have time to look at this in the near future. If you or @hutch120 think you're up for it, feel free to resolve the conflicts and submit a new PR. I haven't even had a chance to see how extensive the conflicts are (could be a fairly simple fix).

@JonathanSeguin JonathanSeguin mentioned this pull request Jul 4, 2017
@JonathanSeguin
Copy link
Contributor

JonathanSeguin commented Jul 4, 2017

@jlbooker See #154 for new PR

@JonathanSeguin
Copy link
Contributor

This was recently merged. Move to close.

@jcrben jcrben closed this Aug 3, 2017
@hongz1
Copy link

hongz1 commented Nov 12, 2017

Thanks guys! Since I added this feature into other repo and haven't check its committed in here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants