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

feat: detect scroll completion with scrollend event #707

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

piecyk
Copy link
Collaborator

@piecyk piecyk commented Apr 16, 2024

As scrollend event is supported by almost all major browser ( without safari ) we can use it to detect scroll completion, in this change the observeElementOffset type signature changed, current it will be not only responsible to return scroll offset but also if is scrolling.

https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollend_event
https://developer.chrome.com/blog/scrollend-a-new-javascript-event/

Copy link

nx-cloud bot commented Apr 16, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 757dd10. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@piecyk piecyk changed the title feat: detect scroll was complete with scrollend feat: detect scroll completion with scrollend event Apr 16, 2024
passive: true,
})
element.addEventListener('scroll', handler, addEventListenerOptions)
element.addEventListener('scrollend', endHandler, addEventListenerOptions)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it makes sense to add event listener even though browser won't support it 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be no op, overall wrapping it with if in two places don't really change anything


return () => {
element.removeEventListener('resize', handler)
}
}

const supportsScrollend =
typeof window == 'undefined' ? true : 'onscrollend' in window

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering why true in case of missing window 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in non window scrollend should be handled by caller, we don't want to create this fallback as it's only for browser environment.

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