Technical review: Document scroll promise return values#44515
Technical review: Document scroll promise return values#44515chrisdavidmills wants to merge 8 commits into
Conversation
chrisdavidmills
requested review from
hamishwillee
and removed request for
a team
chrisdavidmills
changed the title
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ```js | ||
| scrollBtn.addEventListener("click", async () => { | ||
| toolbar.className = "fade-out"; | ||
| await section.scroll(0, 1000); | ||
| console.log("Scroll finished"); | ||
| toolbar.className = "fade-in"; | ||
| }); |
There was a problem hiding this comment.
- The examples don't work yet. IMO would prefer in-page examples to these kinds of link elsewhere examples.
- IFF the return type is a promise that resolves with undefined (as per spec) the example is good. If the promise actually resolves with
interruptedas the docs say then this should output the value instead.
Ditto everywhere.,
There was a problem hiding this comment.
- See Add scroll-promises examples dom-examples#386 for the examples. I've just updated the PR so that the example event handlers output an indication of whether the scroll was interrupted or not, rather than just a static log message.
- I normally agree that in-page live examples are better. However, in this case, you've got seven pages to document this behavior on, and the promise return value, while significant, is not the be-all and end-all of the methods; it is more of a useful addition. I don't really want to repeat the same nearly identical code on seven pages; instead, I've created two examples — one to cover
Element.xand one to coverWindow.x, and just shown the pertinent bits on each page.
I will update the example description on each page to match the changes I've made to the example repo before passing this back to you again.
There was a problem hiding this comment.
OK; descriptions and code snippets updated as per the dom-examples changes.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hamishwillee
left a comment
hamishwillee
left a comment
There was a problem hiding this comment.
This looks good @chrisdavidmills but the example have a flaw mdn/dom-examples#386 (review)
Appreciate you don't want to do in-page examples, but you might still consider EmbedGHLiveSample so that users don't have to leave the page (just a thought, haven't tried it).
Approving so you can merge this and your examples together when the examples are ready.
Thanks, @hamishwillee. I'll wait for your comment on the |
|
|
||
| Load our [element methods demo](https://mdn.github.io/dom-examples/scroll-promises/element-methods/) ([see source code](https://github.com/mdn/dom-examples/tree/main/scroll-promises/element-methods)) in a new tab and click the buttons to see the scrolling behavior. Note how the toolbar fades out when a button is pressed, and fades in again once the smooth scrolling is finished. | ||
|
|
||
| Try pressing one button and then quickly pressing another button before the first scrolling operation has finished. Open your browser's JavaScript console and note how, in these cases, the scrolling is reported as interrupted. |
There was a problem hiding this comment.
May we perhaps use an animation to show the interruption? What if the scroller and/or the button flashes briefly?
4 participants
Description
In Chrome 150 onwards, the standard
ElementandWindowscroll methods now return promises. See https://chromestatus.com/feature/5082138340491264. This is particularly useful when a scroll operation is not instantaneous (e.g. you've gotscroll-behavior: smooth;set on the scrolling container), and you want to programmatically react to the scroll operation finishing, for example, by updating the UI.Affected methods are:
Element.scroll()Element.scrollBy()Element.scrollIntoView()Element.scrollTo()Window.scroll()Window.scrollBy()Window.scrollTo()My tests revealed that only the standard methods return a promise. Non-standard methods (
Element.scrollIntoViewIfNeeded(),Window.scrollByLines(),Window.scrollByPages()) do not, at least not yet.This PR adds content about the promise return values to each relevant method page, and makes some other improvements.
Motivation
Additional details
Related issues and pull requests