Uh oh!
There was an error while loading. Please reload this page.
add 0 Task + carousel, confused in numbers and called a branch of hom… - #19
add 0 Task + carousel, confused in numbers and called a branch of hom…#19olgakuzminagithub wants to merge 5 commits into
Conversation
| * background-color | ||
| * */ | ||
| /* Не получилось */ |
There was a problem hiding this comment.
а здесь дело в том, что получится только при помощи appendChild :)
| list[i].style.left = nextPosition + 'px'; | ||
| let position = parseInt(list[i].style.left); | ||
| if (position === 0) { | ||
| list[i].classList.add('active') |
| } | ||
| start() { | ||
| /*Объявляем перменные */ | ||
| let carousel = document.querySelector(this.elementToApply); |
There was a problem hiding this comment.
that method VERY BIG, can we split some logic to other methods?
| } | ||
| infinityCarousel () { | ||
| let last = this.list.length - 1; | ||
| if (!this.list[last].classList.contains('active')) { |
There was a problem hiding this comment.
there a bunch of calculations, want to think about it optimization a bit
| this.list[i].style.left = nextPosition + 'px'; | ||
| let position = parseInt(this.list[i].style.left); | ||
| if (position === 0) { | ||
| this.list[i].classList.add('active') |
There was a problem hiding this comment.
what about method toggle ? do you remember any ?:)
| this.list[i].classList.remove('active') | ||
| } | ||
| } | ||
| } else if (this.list[last].classList.contains('active') && this.infinity === true) { |
There was a problem hiding this comment.
this.list[last].classList.contains('active') && this.infinity === true
well, having such conditionals is a tech-debt.
We can ask user for set default active element for example to avoid such construction or something else
OlegLustenko
commented
Jul 31, 2018
Here several things to improve but I think it's ready to merge |
…emaking 18