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

scrollTo / scrollToIfNecessary not working for new row at bottom of grid (off screen) #6507

Closed
RobPierce opened this issue Dec 19, 2017 · 3 comments · Fixed by #6644
Closed

Comments

@RobPierce
Copy link
Contributor

RobPierce commented Dec 19, 2017

scrollToIfNecessary no longer works if the grid only extends a few lines below the visible section of the grid - this is causing the percentage calculation for the scrollEvent to be > 1 and a recent code change #6241 put in a check so that if the percentage > 1 then the scrollEvent is not setup. I'm not sure what this change was fixing but it has broken this scenario. If I knew the reason for the original change I would fork and try to fix it but I don't want to mess with it and then break something else that was fixed originally.

Here is a plunker to replicate the issue:

http://plnkr.co/edit/F3aMCzRQmBwSDip2cZ2Z?p=preview

@RobPierce
Copy link
Contributor Author

@rdkleine if you could let me know the original issue that this change was fixing then I'd be happy to have a look at implementing a fix for this that doesn't undo the fix you made?

@RobPierce
Copy link
Contributor Author

@mportuga do you know what the code change #6241 was put in to fix as it has caused this issue but I don't want to work on fixing it without knowing why the change was made in the first place?

@rdkleine
Copy link
Contributor

I have moved on from the project where the grid was needed some time ago.

Ok I'll try to explain from memory:

  1. The bottom row is in edit mode.
  2. Cell navigation is enabled.
  3. Cell receives focus. BUT the cell is not 100% visible.
  4. You start typing. The edit action is cancelled because the grid scrolls the full cell into view.
  5. You are still typing but the first character is gone because what happened in 4.

You can reproduce in tutorial 301
Focus on bottom visible cell type "hello". "h" keypress will activate scroll. "ello" will be the end result.
301 hello

mportuga pushed a commit that referenced this issue Mar 28, 2018
Updated the scrollToIfNecessary to treat any percentage number for the scroll position that is
greater than 1 as 1.

fix #6571, fix #6507, fix #6563
mportuga pushed a commit that referenced this issue Mar 28, 2018
Updated the scrollToIfNecessary to treat any percentage number for the scroll position that is
greater than 1 as 1.

fix #6571, fix #6507, fix #6563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants