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(module:carousel): support dynamic change of nz-carousel-content #60

Merged
merged 1 commit into from
Aug 18, 2017
Merged

fix(module:carousel): support dynamic change of nz-carousel-content #60

merged 1 commit into from
Aug 18, 2017

Conversation

vthinkxie
Copy link
Member

Closes #56

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.163% when pulling dc30074 on vthinkxie:master into ba8af56 on NG-ZORRO:master.

Copy link
Contributor

@giscafer giscafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done!

if (this.nzVertical) {
this._renderer.setStyle(this.slickList.nativeElement, 'height', `${this.hostElement.nativeElement.offsetHeight}px`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One line of code is missing here.
this._renderer.removeStyle(this.slickList.nativeElement, 'height');

if not,in vertical mode, the slickList's height will be setting to 0 .

Copy link
Member Author

@vthinkxie vthinkxie Aug 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slickList's height is defined when ngAfterViewInit trigger, this height won't change when vertical mode. If remove its style here, the height will be multiple here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vthinkxie if not, the height is 0, nz-demo-carousel-vertical demo can't display normally

you should test it by modify the code:

NzDemoCarouselVerticalComponent

export class NzDemoCarouselVerticalComponent implements OnInit {
 array = [  ]; // try dynamic change the array
  
    constructor() {
    }
  
    ngOnInit() {
  setTimeout(()=>{
      this.array = [ 1, 2, 3, 4 ];
    },500)
    }
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. I will update this code.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.163% when pulling d69bed6 on vthinkxie:master into 0c6bcbb on NG-ZORRO:master.

@vthinkxie vthinkxie merged commit 44865c2 into NG-ZORRO:master Aug 18, 2017
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.

3 participants