Carousel: new module (WIP) #668
Conversation
|
For the slideshow carousel in the current implementation there is no required width. As it is right now it works the same way as the items-per-slide carousel. You can have 2 items-per-slide in a slideshow for example. |
|
Okay, let's chat offline. The term slideshow is a term I've been using (correctly or incorrectly) specifically for a carousel with 1 item in the viewport. For me, it's analogous to watching a photo slideshow on a projector, or a powerpoint slideshow, where we see 1 item at a time. Let me have a think and see if that analogy still holds up with how we have things in coreui (I may be blurring the lines between items and slides). In the meantime, if you get a chance to fix the CSS for example 3 in this branch, that would be most appreciated. |
|
Analogy refinements:
|
|
Okaaay.... hopefully 3rd time lucky. @DylanPiercey @seangates have a gander at this. After our conversation, and looking through coreui examples, I realised that the concept of a "slide" is a little blurry, and in fact sometimes they don't even exist. I have reworked the docs so that they fit well with our current implementation in coreui (and should still work well with any implementation) and still fits okay with MIND patterns terminology. I didn't change or add any classnames in the end. And @DylanPiercey, I'd still like you to take a quick look at the gap issue between carousel and dots (screenshots at end)
Broken dots:
|
|
@ianmcburnie regarding the broken dots, it looks like actually all the carousel's are broken (look at the paddles which are not centered). Looks like this is related to the negative margin hack to hide the scroll bar, but I'm not yet sure why this is happening only in skin 🤔 |
|
Still not sure how this is different. But it looks like we can fix it by removing the negative margin from the .carousel__container and putting it on the .carousel__list. Then removing the margin from the paddles. I'm not sure why I put this on the container, but it still appears to be properly hiding the scroll bar with this approach 😕. |
|
Ok, I just tried your suggestions thanks, but the position of the paddles is different depending on whether the dots are present or not. Wonder if we need a |
|
@ianmcburnie did you remove the margin on the paddles? https://github.com/eBay/skin/pull/668/files#diff-49c91f2e5e7709f03559710ce2c1482fR206 I thought it looked right after doing that |
|
Yep I did. With dots, the paddles seem low down:
Compared to when there is no dots:
Tested in Safari. |
|
Oh! I had the dots html within the container div. My bad. |
Closes #131
This is a DRAFT pull request as I need some assistance from @DylanPiercey.
I copied over the styles from coreui like for like. The only change I made was to alphabetize the property order for stylelint.
Overall things are looking good. I added 3 examples to the website for DS4 and DS6:
In that last example, with dots, there is a spacing issues between the carousel and the dots (see screenshot below). While I could solve this myself, I would prefer Dylan to take first look as he is more familiar with the nuances of why the styles are the way they are. I don't want to unwittingly cause a regression elsewhere.
For example 2 I also have a question regarding whether my statement is true:
The list items in a slideshow are typically set to 100% width.I'm assuming it is, and if so should we not be setting the
lito 100% width? Currently we do not, and I had to set it in the demo styles.