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

Fix bad styles being cached in massive lists #527

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

nickclaw
Copy link
Contributor

@nickclaw nickclaw commented Jan 6, 2017

In lists large enough to trigger scaling, the cached styles can be invalid. Causing weird banding effects and overlapping as you scroll.

bands

Copy link
Owner

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix.

Supporting this feature (max scroll offset) is a huge pain in the ass. I wish I didn't have to support it. :/

height: 100,
rowHeight: 100,
columnWidth: 100,
rowCount: DEFAULT_MAX_SCROLL_SIZE * 2 / 100, // lots of offset
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an arbitrary/odd rowCount value. What's the logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the height of the inner container needs to be over DEFAULT_MAX_SCROLL_SIZE to trigger this. Doing DEFAULT_MAX_SCROLL_SIZE / rowHeight will get us exactly enough rows to fill the container -- so I figured I might as well double it to be sure :P

I'm happy to add comments explaining it, or if you have a better suggestion for the test use that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I think the order of operations threw me a bit. (It's late here.) It's fine as is. :)

@bvaughn bvaughn merged commit 8906f30 into bvaughn:master Jan 6, 2017
@nickclaw
Copy link
Contributor Author

nickclaw commented Jan 6, 2017

I can tell haha, writing the ScalingCellSizeAndPositionManager alone looked like enough to make one go crazy

@bvaughn
Copy link
Owner

bvaughn commented Jan 6, 2017

I have hopes that the new layout algorithm in PR #515 might totally change the way scaling is done- making things like offset-adjustments and ScalingCellSizeAndPositionManager unnecessary. But it's still kind of an experiment. It might not pan out.

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

Successfully merging this pull request may close these issues.

2 participants