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

Bug in Carousel for external Previous/Next buttons #2766

Closed
smichelotti opened this issue Oct 5, 2018 · 2 comments
Closed

Bug in Carousel for external Previous/Next buttons #2766

smichelotti opened this issue Oct 5, 2018 · 2 comments

Comments

@smichelotti
Copy link

Bug description:

Steps to reproduce:

  1. Click "Next" button - observe nothing happens.
  2. Move your mouse into the image - observe the Carousel advance on mouseenter.

Other Notes:

  • If you click the Next button twice, then mouseenter the image, it will advance 2 images (corresponding exactly to number of clicks). It's correctly keeping track of active slide, but not advancing until mouse enter.
  • Same behavior (in reverse) for Previous button.

Link to minimally-working StackBlitz that reproduces the issue:

StackBlitz sample that reproduces issue: https://stackblitz.com/edit/ng-bootstrap-carousel-bug

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 6.1.0

ng-bootstrap: 3.3.0

Bootstrap:

@pkozlowski-opensource
Copy link
Member

The issue is here:

map(() => this.interval), filter(interval => interval > 0 && this.slides.length > 0),

probably a regression introduced in e0c0050#diff-ffce1c981a98a34d71e247f67d00f7cd

maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue Oct 10, 2018
Regression introduced after marking carousel OnPush due to lack of tests

Fixes ng-bootstrap#2766
@maxokorokov
Copy link
Member

Yep it's a regression after moving carousel to OnPush, there were no tests for API calls like prev(), next() and select().

It isn't related to interval 0, same issue with any interval value → calling API methods in question wasn't switching slides immediately.

maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue Oct 11, 2018
Regression introduced after marking carousel OnPush due to lack of tests

Fixes ng-bootstrap#2766
maxokorokov added a commit that referenced this issue Oct 11, 2018
Regression introduced after marking carousel OnPush due to lack of tests

Closes #2776
Fixes #2766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants