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

allow scrolling past child layer, when a new scroll starts at the end of child layer #465

Closed

Conversation

@gterzian
Copy link
Member

gterzian commented Oct 22, 2016

WIP for servo/servo#13249


This change is Reviewable

@gterzian gterzian changed the title allow scrolling pas child layer, when a new scroll starts at end allow scrolling past child layer, when a new scroll starts at end Oct 22, 2016
@gterzian gterzian changed the title allow scrolling past child layer, when a new scroll starts at end allow scrolling past child layer, when a new scroll starts at the end of child layer Oct 22, 2016
@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch 11 times, most recently from 7d38381 to b79cac5 Oct 23, 2016
@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch 2 times, most recently from 5c026ca to 5618b78 Oct 30, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2016

The latest upstream changes (presumably #488) made this pull request unmergeable. Please resolve the merge conflicts.

@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch from 5618b78 to 685b2af Oct 31, 2016
@glennw
Copy link
Member

glennw commented Oct 31, 2016

@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch from 685b2af to 87ffa1d Oct 31, 2016
@gterzian
Copy link
Member Author

gterzian commented Oct 31, 2016

To summarize what was discussed in servo/servo#13249

  • "If a new scroll gesture starts while you're already scrolled all the way to the edge of the scrollable element under the mouse, then the scroll will be handed off to the next outer scrollable element which is actually scrollable in that direction" servo/servo#13249 (comment)
  • "If you scroll a long page, and somewhere on that page is a smaller scrollable element, you don't want that element to swallow the rest of your scroll gesture if it suddenly happens to move under your mouse. You want the current scroll gesture to keep scrolling the outer page." servo/servo#13249 (comment)
@pcwalton
Copy link
Collaborator

pcwalton commented Nov 1, 2016

Is this ready for review and ready to go?

@gterzian
Copy link
Member Author

gterzian commented Nov 2, 2016

@pcwalton yes it is, thank you for your time.

@gterzian
Copy link
Member Author

gterzian commented Nov 3, 2016

@pcwalton since from the issue itself it might not be clear what the goal of this PR is, there are two:

  1. When a child layer has scrolled to it's end, and a new scroll gesture is started, start scrolling the nearest parent layer. Current behavior: one is stuck inside the child layer.
  2. When one is scrolling a layer, and one encounters a scrollable child layer, one should keep scrolling the currently scrolling parent layer. Current behavior: the scroll is 'captured' by the child layer, which starts scrolling.

See the comments above for links to the discussions around this within the issue.

@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch 5 times, most recently from 2e46231 to bb584cf Nov 3, 2016
@gterzian gterzian force-pushed the gterzian:allow_scroll_past_end_of_child_layer branch from bb584cf to 3b33c0e Nov 3, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Nov 4, 2016

The latest upstream changes (presumably #517) made this pull request unmergeable. Please resolve the merge conflicts.

@gterzian
Copy link
Member Author

gterzian commented Nov 4, 2016

@pcwalton @glennw Since there was quite a big change in the scroll function, I am currently looking if my changes are necessary at all, and if so, how to re-implement based on the latest... I'll let you know when that is done...

@mrobinson
Copy link
Member

mrobinson commented Nov 4, 2016

@gterzian Let me know if you need my input for that. I'm sorry that my change stomped on yours!

@gterzian
Copy link
Member Author

gterzian commented Nov 4, 2016

@mrobinson thanks for your offer of input, and no problem at all! 😄

Let me get a better look at the changes, and test the latest behaviour, then I'll get back to you...

@gterzian
Copy link
Member Author

gterzian commented Nov 6, 2016

@pcwalton @glennw ok from my initial testing it seems that the behavior suggested in #465 (comment) is still something that needs to be implemented.

@mrobinson perhaps it's best if I wait until the servo part of you work has been merged, and then use that as a basis for exploring how to re-implement my changes?

@gterzian
Copy link
Member Author

gterzian commented Nov 6, 2016

closing, in favor of #533

@gterzian gterzian closed this Nov 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.