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

returnIndex value not correct on multiple slides #197

Open
aderaaij opened this issue Feb 9, 2016 · 8 comments
Open

returnIndex value not correct on multiple slides #197

aderaaij opened this issue Feb 9, 2016 · 8 comments
Assignees
Labels

Comments

@aderaaij
Copy link

aderaaij commented Feb 9, 2016

Heya! Like I already mentioned on Twitter: Love the slider and lack of compromises to make it nice and modern. Also loving the events, but I do have a question on those though.

I make a lot of use of returnIndex, but it seems that it doesn't return the right number in the following case:

When you have a slider with multiple items to slide, i.e. 'slidesToScroll:4' but an amount of slides that doesn't fill out the entire slider, i.e. 10. In this case you'd have 3 'pages', two with 4 items and one with 2 items. Unfortunately, on the last page, the 'active' class stays stuck on the 5th item (returnIndex = 4) which makes little sense to me as this is out of the viewport. Also, it makes that clicking the back button slides back from slide #5 instead of #9, resulting in a weird slide back (one click vs the expected 2) If I up the amount of slides to 12 the behaviour is as expected.

Is this intended or might this be a bug?

Thanks for your time!

@meandmax
Copy link
Collaborator

I will look into that asap, sorry for the late answer ... might be a bug.

@meandmax meandmax added the bug label Feb 21, 2016
@leonardofaria
Copy link

@aderaaij could you post your example?

@aderaaij
Copy link
Author

@leonardofaria I'll set up a demo asap, will report back!

@riotcku
Copy link

riotcku commented Jun 8, 2016

I have a similar issue, but a simpler one.

When I use returnIndex() at the last slide (length - 1) with the rewind option as true, I get the wrong index, specifically it returns the index of the previous slide. If this is a separate bug from this issue, please let me know and I'll make a separate issue.

@riotcku
Copy link

riotcku commented Jun 22, 2016

@meandmax @aderaaij Figured the bug out.

This is because Lory uses an offset for calculation of the next index. HTML adds 4px margin between every element that are inline-block with whitespace. This causes this conditional statement

if (slides[nextIndex].offsetLeft <= maxOffset) {
    index = nextIndex;
}

to not fire, not updating the index.

Solution is either using CSS (One simple workaround could be a selector of non-first-child that applies margin-left: -4px) or a more permanent solution would be for Lory to not use offset as its counter for index.

@pieplu
Copy link

pieplu commented Mar 29, 2017

Solution is either using CSS (One simple workaround could be a selector of non-first-child that applies margin-left: -4px) or a more permanent solution would be for Lory to not use offset as its counter for index.

@riotcku
Or a font-size:0 on the parent? The 4px depends of the font-size

@riotcku
Copy link

riotcku commented Mar 31, 2017

@pieplu Yup, 4px is the magic number for most sites because the unicode whitespace width is .25em and 16px is the default font-size.

font-size: 0 on the parent is a good fix that gets around this but browsers nowadays have a minimum font size! So this fix is not reliable as well.

In my personal case, solution was to use the CSS3 white-space-collapsing property but this is because I don't need to worry about non-css3 supporting browsers.

The best fix would be to solve for this in Lory instead of CSS methods.

@mikemaccana
Copy link

mikemaccana commented Jun 13, 2018

Same here. I'm just hacking it for now - long term it would be better to move away from inline-block completely - slides aren't really blocks inside a line of characters - and just use grid, now most people are on an evergreen browser.

// HACK
// 1050 highlights last item but no rewind
// 1025 doesn't highlight last item
// See https://github.com/meandmax/lory/issues/197
const FIX_BUG = 1040

const maxOffset = Math.round(slidesWidth - frameWidth) + FIX_BUG;

mikemaccana pushed a commit to mikemaccana/mikemaccana.com that referenced this issue Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants