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 resize-scroll when grid is not at top of page #1727

Merged
merged 2 commits into from
Apr 13, 2021

Conversation

Manfred-on-github
Copy link
Contributor

@Manfred-on-github Manfred-on-github commented Apr 12, 2021

Description

when scroll element does not start from top of page, the scrolling during resize does not work as expected, the decision if to scroll, and amount to scroll, is computed wrong.
This PR fixes this.

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • Extended the README / documentation, if necessary

@adumesny
Copy link
Member

@Manfred-on-github did you test with latest 4.2.0 as I made some changes to scrollbar during dropover (fix #1704) which may or may not be related... would be great to have an example showing the issue before.

thanks for doing this fix!

@Manfred-on-github
Copy link
Contributor Author

have added a test page. Have checked error can be reproduced in 4.2.0, 4.0.3 and 3.3.0.

@adumesny adumesny merged commit 6dcbac2 into gridstack:master Apr 13, 2021
@adumesny
Copy link
Member

thanks, that is perfect.

@Manfred-on-github Manfred-on-github deleted the fix-resize-offset-top branch April 13, 2021 14:34
@adumesny
Copy link
Member

funny you called it 'gridster' :) did you used that lib before ??

@Manfred-on-github
Copy link
Contributor Author

oh - yes, we used that originally, then, looking for something that is still maintained, we found gridstack. - Thank you for keeping this up.

@Manfred-on-github Manfred-on-github changed the title fix resize-scroll when gridster is not at top of page fix resize-scroll when gridstack is not at top of page Apr 13, 2021
@adumesny
Copy link
Member

while this fixes the offset issue of the grid, still doesn't work perfectly as it scroll too soon... why does it scroll when we hit the white sidebar and not the actual end of the yellow grid ?

@adumesny adumesny changed the title fix resize-scroll when gridstack is not at top of page fix resize-scroll when grid is not at top of page Apr 13, 2021
@Manfred-on-github
Copy link
Contributor Author

for me it scrolls when the y-position of the pointer moves outside the range indicated by the white sidebar.

This seems to be by design, as the function updateScrollResize() is invoked from gridstack-dd.ts with parameter distance as cellHeight, therefore (to visually show this in my sample) have explicitly set grid rowHeight to 130px, which is also used as border height in the sidebar.

Of course this might be changed to some smaller value. For me it is also ok as it is now, as widgets further down will appear in view early before the resize just pushes them down, so user better sees what resizing of a particular widget will mean to positions of other widgets.

Something that would rather improve user experience, is connected to the comment in that function about using a timer. As it is now, when pointer gets near the edge, this function scrolls the page, but then the pointer is still near the edge, so condition to scroll still holds, and the next scroll happens immediately, and so on. So something that limits these scroll events would definitely help here.

@adumesny
Copy link
Member

adumesny commented Apr 13, 2021

yes about timer, and what #1576 is about (at least the speed part which I agree and needs to get done).

| For me it is also ok as it is now, as widgets further down will appear in view early before the resize just pushes them down, so user better sees what resizing of a particular widget will mean to positions of other widgets.

I can see that, but it's odd if you don't have widgets directly below to see what will happen on next block jump. Otherwise it's odd to scroll before you reach the edge.

@adumesny
Copy link
Member

adumesny commented Apr 23, 2021

@Manfred-on-github looks like your change caused #1745 issue. I forced offsetTop = 0 (revert your change) and back to working as expected...

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.

None yet

2 participants