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

Fix caret when currently focused keyboard is refocused with no preview window #394

Merged
merged 1 commit into from Sep 24, 2015

Conversation

insightfuls
Copy link
Contributor

The code you had in here to fix the caret didn't work without a preview window. I've adjusted and the new code works for my use case. Do you think the change is appropriate? We could still do find() with another context I suppose, though I guess using base.$caret will be faster, and I don't see any reason it shouldn't reference any existing caret.

@insightfuls
Copy link
Contributor Author

Just realised I forgot to run the build before committing this, so the combined/minified sources aren't updated. Sorry.

@Mottie
Copy link
Owner

Mottie commented Sep 24, 2015

Hmm, that code should have worked even without a preview since it uses the $keyboard to look for the caret. Odd.

I had it set up that way because during development, if I remember correctly, I was ending up with multiple carets in some situations... when stayOpen or alwaysOpen are set maybe? LOL I really need to add some unit testing, but now I'm just too lazy/busy.

@insightfuls
Copy link
Contributor Author

The caret isn't added within $keyboard when there is no preview. It is always added to $preview, but when there isn't truly a preview, $preview==$el which, of course, isn't within $keyboard. So I suppose $preview.find() would work, but I think base.$caret is better/more efficient, if it works, and I think it does.

Multiple carets is the problem I was experiencing (and I do use stayOpen which may indeed be related). It happened when I focused an already-focused input. It seems caret_setup would get called before/without the teardown of the caret, so the old caret would remain.

Anyway, this change fixes it for me, and I don't see any reason it wouldn't also fix the problem you were experiencing, especially since it sounds like we were experiencing the same one.

@Mottie
Copy link
Owner

Mottie commented Sep 24, 2015

Ok, sounds good.

Mottie added a commit that referenced this pull request Sep 24, 2015
Fix caret when currently focused keyboard is refocused with no preview window
@Mottie Mottie merged commit 8912c48 into Mottie:master Sep 24, 2015
@Mottie
Copy link
Owner

Mottie commented Sep 24, 2015

Oh, and the caret does work when you set usePreview to false... here is a demo.

@insightfuls
Copy link
Contributor Author

O, the caret definitely works in general, including with usePreview as false; it was just the fix for the multiple caret problem that was broken. (Ironically, the JSFiddle demo doesn't work for me, but that's probably a connection problem or something, as even the HTML component doesn't seem to work.)

@Mottie
Copy link
Owner

Mottie commented Sep 25, 2015

That demo doesn't work if you have the protocol set to https... it's because the links to the files use http.

@Mottie
Copy link
Owner

Mottie commented Sep 25, 2015

Even though that demo is using protocol relative links, GitHub is switching the protocol to http causing the problems.

@Mottie
Copy link
Owner

Mottie commented Sep 25, 2015

Duh... the links were set to github.com and not github.io.

https://jsfiddle.net/Mottie/egb3a1sk/793/

Odd, that jsFiddle it still won't load jQuery UI javascript... so everything works except for the positioning of the keyboard.

@insightfuls
Copy link
Contributor Author

That probably explains it. Not to worry, we both know it works anyway. :-)

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.

None yet

2 participants