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

Carousel key events #14991

Closed
pipja opened this issue Nov 4, 2014 · 9 comments · Fixed by #14993
Closed

Carousel key events #14991

pipja opened this issue Nov 4, 2014 · 9 comments · Fixed by #14993
Labels
Milestone

Comments

@pipja
Copy link

pipja commented Nov 4, 2014

Hi, I have a little problem with Carousel key events.
Whenever the cursor is in a text-box on my slide and I press the left or arrow key, instead of the cursor moving between characters, Carousel will catch the event and move the slide next or prev. Taking out the Carousel keydown events from bootstrap.js solve the issue but I am not allowed to make direct changes to third party libraries.
Just wondering if this is a bug or not, and maybe the Carousel keydown events should be disabled by default because passing data-keyboard="false" does not work :(.

@cvrebert
Copy link
Collaborator

cvrebert commented Nov 4, 2014

data-keyboard="false" works, I just re-tested it. Presumably you're not using Bootstrap v3.3.0?

@cvrebert
Copy link
Collaborator

cvrebert commented Nov 4, 2014

Although perhaps we ought to have Carousel's keyboard listener ignore keyboard events fired at <inputs>, <textarea>s, and the like.

(Even though I don't think we'd particularly recommend putting forms in carousels)

@cvrebert cvrebert added the js label Nov 4, 2014
@pipja
Copy link
Author

pipja commented Nov 4, 2014

yes I think the keyboard listener needs to ignore keyboard events fired from inputs and textarea. If i click outside the textbox the events don't fire, but as soon as I click inside the box and press the keys carousel will go crazy.

PS: I think I am currently on 3.2.0

@pipja
Copy link
Author

pipja commented Nov 4, 2014

I actually have a very cool form-based wizard-like carousel, that's why the misfired key events

@hnrch02
Copy link
Collaborator

hnrch02 commented Nov 4, 2014

I feel like this has come up before. The simplest solution is just to do this:

$('#myCarousel').on('keydown', 'input', function(e) {
  e.stopPropagation();
});

I don't think we should put a workaround for that in the core, since we don't advise to put anything else than text in carousels.

@pipja
Copy link
Author

pipja commented Nov 4, 2014

We did not initiate the creation of carousel from javascript, and we do not use jquery (we use angularjs), so putting that line anywhere in our self-written javascript will incur massive scrutiny.

Also from my pov, Carousel catching textbox and textarea events is a bug in Carousel itself

@hnrch02
Copy link
Collaborator

hnrch02 commented Nov 4, 2014

The carousel was not designed for that purpose and therefore you will need to work around it in a inconvenient way.

Anyhow, what do you mean by this:

We did not initiate the creation of carousel from javascript, and we do not use jquery (we use angularjs)

If you are using https://github.com/angular-ui/bootstrap then you should open the issue with them since that codebase is completely decoupled from Bootstrap.

@cvrebert
Copy link
Collaborator

cvrebert commented Nov 4, 2014

If you're using Angular, then you should probably be using http://angular-ui.github.io/bootstrap/ or similar instead of Bootstrap's regular JS.
[Edit: Ah, the wonders of using mobile and not seeing comments posted while composing one's own comment.]

@pipja
Copy link
Author

pipja commented Nov 4, 2014

no no no, our entire java base was written in angularjs, but carousel was using full-blown bootstrap.js. And using bootstrap.js already had me going through a full round of hurt for using jquery stuff. I am not using angular-ui.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants