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

List background swaps when scrolling #18

Open
lfuelling opened this issue Dec 29, 2015 · 8 comments
Open

List background swaps when scrolling #18

lfuelling opened this issue Dec 29, 2015 · 8 comments
Assignees

Comments

@lfuelling
Copy link

Hi,

I've just started using play-midnight (the latest version from the chrome web store) and I stumbled upon a small issue with the background color of playlists.

The color keeps swapping (light and dark) when scrolling past the first few songs.

Here's a screencap to visualize what I mean:
bugmidnight

@ducky
Copy link
Owner

ducky commented Dec 29, 2015

That's actually really weird, haha. I just looked into it and I see why, though. Sadly, there's nothing I can do easily (that I know of) to fix that.

It's because I'm doing the zebra stripes (that Google doesn't do) and the way their playlist scroller works, it's actually removing and adding items as you scroll, which is causing the nth-child css selector to swap between as the items get added and removed.

The only way I know to fix it would be to manually add a class to the rows every time you go to a page which would probably just make Play Music slower than it already is. Or possibly add an option to disable zebra stripes.

Also, appreciate the visual reference, super helpful! 👍

@ducky ducky self-assigned this Dec 29, 2015
@ducky ducky added the bug label Dec 29, 2015
@geudrik
Copy link

geudrik commented Jan 6, 2016

I wouldn't call this a bug - the cause is out of your control. The even-odd class attributes are being added by Googles libs as you scroll, which gives the alternating row styles.

@chrisxclash You could look at the length of the table each time new rows are added and add an empty row if the number of rows would cause a background change

@geudrik
Copy link

geudrik commented Jan 6, 2016

screen shot 2016-01-06 at 4 15 56 pm

<tbody data-count="34" data-start-index="7" data-end-index="34"> is the important bit. Just watch for changes in data-count, data-start-index, and data-end-index and modify as necessary to prevent the JS from auto-formatting the rows. The downside of doing it this way would be that you'll need to undo those changes when new rows are added (i think - I haven't tested).

These would the same attributes you monitor (onChange even) and add an empty row if the rowcount becomes its inverse (even vs. odd total count)

@ducky
Copy link
Owner

ducky commented Jan 6, 2016

@geudrik - Yeah, I definitely had that idea in mind, but it seems like overkill for something as simple as zebra stripe styles. I didn't feel the benefit would outweigh the extra JavaScript that would have to watch and swap classes all the time since Play Music is already slow.

It seems really weird in general how they're using a heightened placeholder and just swapping rows, but I can see it be beneficial performance-wise, idk.

@geudrik
Copy link

geudrik commented Jan 6, 2016

\shrug totally understand your hesitance here. I agree that it's probably not worth the extra effort

Just figured I'd throw my ideas out 😄

@ducky
Copy link
Owner

ducky commented Jan 6, 2016

Haha yeah. I'll keep it open for now as a reminder.

I'm currently in the works of rewriting all my JavaScript anyways, so I'll have to look into it when I'm putting everything else together.

@ducky ducky added the styling label Apr 15, 2016
@erickrawczyk
Copy link

Is there any way to turn off the zebra stripes? I find the swapping when scrolling to be more detrimental to the experience than including them.

@ducky
Copy link
Owner

ducky commented Sep 11, 2018

@erickrawczyk - Currently no, but at this point I think there definitely should be. I'll add a note to add an option for disabling it when I get to working on the next update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants