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

Not loading data when zoom set to < 100% #199

Closed
les2 opened this issue Oct 4, 2016 · 8 comments
Closed

Not loading data when zoom set to < 100% #199

les2 opened this issue Oct 4, 2016 · 8 comments

Comments

@les2
Copy link

les2 commented Oct 4, 2016

For some reason, the table is not loading the first page of data when the user has the zoom set to 90% (technically, any zoom < 100%).

We are using ELT without a fixed header and without a height set on the table.

I will see if i can create a repo with a simple, reproducible example later on.

Not sure where the bug is - going to attempt to debug it shortly.

@buschtoens
Copy link
Collaborator

Can you tell us which browser and OS or does it happen in any browser?

@offirgolan
Copy link
Collaborator

offirgolan commented Oct 5, 2016

@les2 I'm not able to recreate this in the demo app (zoom set to 75%).

If you add a fixed header + height does it solve your issue?

@les2
Copy link
Author

les2 commented Oct 5, 2016

@buschtoens debugged issue with one of our users and i reproduced on chrome 53; working on a reproduction recipe.

@les2
Copy link
Author

les2 commented Dec 3, 2016

update: i've been trying to reproduce the problem over here but no luck yet.

but i can consistently reproduce in one of our production applications. i think i'm using a new version of ELT in the repo i linked above so i'm going to try to reproduce with the exact version we're currently using in production - 1.4.2 - and see if that's the issue.

@les2
Copy link
Author

les2 commented Dec 3, 2016

TL;DR -- this is a floating point bug ... in ember-in-viewport addon.

This is really hard to reproduce because you need the numbers to round just right.

Details

The bug is in one of these two locations:

Example.

// values for bounding client rect of the `lt-infinity` element!
top:  318.85418701171875 left:  0 bottom:  318.85418701171875 right:  1451.1112060546875

// tolerance values:
topTolerance:  0
leftTolerance:  0
bottomTolerance:  500
rightTolerance:  0

// width and height of the window (viewport):
width:  1451 height:  820

// ALL of these have to be TRUE in order for the lt-infinity element to be *in* the viewport:
(top + topTolerance)       >= 0                   ? true
(left + leftTolerance)     >= 0                     ? true
(bottom - bottomTolerance) <= height    ? true
(right - rightTolerance)   <= width           ? FALSE     <---- this is the problem!

right boundary check in detail:

(right - rightTolerance)   <= width
1451.1112060546875 <= 1451    ===> FALSE

So we need to round (down) to integer value the bounding client rect values, I guess.

I'm going to look at a PR to the ember-in-viewport repo in a little bit and then open a PR to this repo that just bumps the version of that package.

@les2
Copy link
Author

les2 commented Dec 3, 2016

@offirgolan I have managed to make it reproducible: https://les2.github.io/ember-light-table-zoom-bug/#/elt-demo

  1. Go to this URL
  2. Set your zoom to 90% (or whatever)
  3. Refresh the page -- notice the initial onScrolledToBottom doesn't fire (because of the ember-in-viewport bug described above)
  4. If you resize the browser window manually, this will set the width of the window to an integer value and the onScrolledToBottom event will fire correctly.

@offirgolan
Copy link
Collaborator

@les2 this seems as though it was one hell of a bug hunt. Thanks for taking the time to put this all together. Im really sorry I have been of no help! Im currently traveling out of the country so I don't have too much time but it seems as though you have a pretty good handle on this issue. I look forward to a PR with a solution on the ember-in-viewport side 😄!

If you have something you wanna run by me, feel free to ping me on slack as I'm a bit more reachable there.

@offirgolan
Copy link
Collaborator

@les2 a new patch (1.8.1) has been released with the latest release of ember-in-viewport. Thanks again for taking the time to hunt this issue down 😸

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

No branches or pull requests

3 participants