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

Smarquee doesn't work for position:absolute #2

Open
YuriGor opened this issue Apr 11, 2019 · 10 comments
Open

Smarquee doesn't work for position:absolute #2

YuriGor opened this issue Apr 11, 2019 · 10 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@YuriGor
Copy link

YuriGor commented Apr 11, 2019

Here is a codepen to reproduce
https://codepen.io/yurigor/pen/wZeBpO?editors=0110
Notice this css code:

.smarquee {
  position:absolute;

if you will comment this line and force codepen to rerun, it will fix the problem.
I guess some initial calculations fail in case of the absolute position of the element, because when I change 'position' on the fly - everything keeps working.

@YuriGor
Copy link
Author

YuriGor commented Apr 11, 2019

Problem is here:

this.marqueeContainer.scrollWidth > this.marqueeContainer.clientWidth

Workaround - to make the width of an element not 'auto' like this:

.smarquee {
  position:absolute;
  right:0px;
  left:0px;

@BuddyLReno
Copy link
Owner

Ah I see. Thanks for the helpful codepen! I think adding the right and left css attributes should be added as initial attributes so the needsMarquee function will still calculate properly.

@BuddyLReno BuddyLReno added bug Something isn't working good first issue Good for newcomers labels Apr 12, 2019
@BuddyLReno
Copy link
Owner

Playing with it a little and what happens is that position: absolute; causes the initial calculation of scrollWidth and clientWidth to be the same. Changing the needsMarquee comparison to be >= allows absolutely positioned elements to get properly initiated with Smarquee.

@BuddyLReno
Copy link
Owner

This should take care of it @YuriGor. If it doesn't, please re-open the issue!

@YuriGor
Copy link
Author

YuriGor commented Apr 12, 2019

Ok, thank you, I'll test it and report here.

@YuriGor
Copy link
Author

YuriGor commented Apr 12, 2019

Fixed version works unstable.

I updated Codepen to use the latest version from Gti:
https://codepen.io/yurigor/pen/wZeBpO
Try to run it several times, sometimes it works, sometimes doesn't.
I logged this.marqueeContainer.scrollWidth and this.marqueeContainer.clientWidth to console:

14404 14404
14400 14400
14397 14398
14402 14402
14405 14406
14396 14397
14401 14401

So, as you see, sometimes widths are equal and animation starts,
but sometimes scrollWidth is less than clientWidth and check fails.

I am not sure where is reopen button, probably I have no permission for it, hope you'll be notified about this comment, @BuddyLReno.

@BuddyLReno BuddyLReno reopened this Apr 12, 2019
@BuddyLReno
Copy link
Owner

@YuriGor ah I see now. Hhmmmm maybe we leave the default calculation as >= but then include an offset value that can be overridden. Maybe a 3px offset so the calculation will read:

scrollWidth >= clientWidth - offset

I'll make this change and push it up to a branch that maybe you could try out and see if it works before I publish a new version.

@YuriGor
Copy link
Author

YuriGor commented Apr 12, 2019

Looks like we don't understand clearly what's happening so we will not be sure issue is really fixed.
Why this +/1pixel shift is happening?
Does it depend on screen size?
Maybe it's related to rounding some fractional size/duration somewhere?
What about an option to force animation without widths checking? What will happen if the contents of the marquee are smaller than container?

@YuriGor
Copy link
Author

YuriGor commented Apr 12, 2019

Documentation for scrollWidth says:

This property will round the value to an integer. If you need a fractional value, use element.getBoundingClientRect()

Maybe this is an answer?

@BuddyLReno
Copy link
Owner

I really like the idea of also adding a force animation option so it doesn't do the needs marquee checking. Yeah I think digging into the docs for both scrollWidth and clientWidth can hopefully reveal some ideas. I'll look at experimenting with the getBoundingClientRect. I like what the docs are saying about what values this gives us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants