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.

Sign up for GitHub

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: add lazy option to allow recalculating targets #388

Merged
merged 4 commits into from
Oct 28, 2020
Merged

feat: add lazy option to allow recalculating targets #388

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb11b5f
feat: add dynamicPosition parameter (for dynamic container height)
RealGoodProgrammer Oct 16, 2020
2bb2cbb
fix: rename option and small refactoring
RealGoodProgrammer Oct 27, 2020
eaecf92
chore: apply suggestions from code review
rigor789 Oct 28, 2020
70254d2
perf: remove redundant calculation
rigor789 Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ In case you are using the browser version (directly including the script on your
VueScrollTo.setDefaults({
container: "body",
duration: 500,
dynamicPosition: true,
easing: "ease",
offset: 0,
force: true,
Expand Down Expand Up @@ -125,6 +126,7 @@ If you need to customize the scrolling options, you can pass in an object litera
el: '#element',
container: '#container',
duration: 500,
dynamicPosition: true
easing: 'linear',
offset: -200,
force: true,
Expand All @@ -151,6 +153,7 @@ var VueScrollTo = require('vue-scrollto');
var options = {
container: '#container',
easing: 'ease-in',
dynamicPosition: true,
offset: -60,
force: true,
cancelable: true,
Expand Down Expand Up @@ -191,6 +194,11 @@ The duration (in milliseconds) of the scrolling animation.

*Default:* `500`

#### dynamicPosition
Recalculating targetY/targetX at each scroll step. Useful when you don't know the final height of the container, and it grows as it scrolls.

*Default:* `false`

#### easing
The easing to be used when animating. Read more in the [Easing section](#easing-detailed).

Expand Down
34 changes: 31 additions & 3 deletions src/scrollTo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const abortEvents = [
let defaults = {
container: 'body',
duration: 500,
dynamicPosition: false,
easing: 'ease',
offset: 0,
force: true,
Expand All @@ -33,6 +34,7 @@ export const scroller = () => {
let element // element to scroll to
let container // container to scroll
let duration // duration of the scrolling
let dynamicPosition //checks the target position at each step
let easing // easing to be used when scrolling
let offset // offset to be added (subtracted)
let force // force scroll, even if element is visible
Expand All @@ -52,6 +54,9 @@ export const scroller = () => {

let abort // is scrolling aborted

let cumulativeOffsetContainer
let cumulativeOffsetElement

let abortEv // event that aborted scrolling
let abortFn = e => {
if (!cancelable) return
Expand Down Expand Up @@ -95,6 +100,24 @@ export const scroller = () => {
if (abort) return done()
if (!timeStart) timeStart = timestamp

// When a site has a lot of media that can be loaded asynchronously,
// the targetY/targetX may end up in the wrong place during scrolling.
// So we will check this at each step
if (dynamicPosition) {
cumulativeOffsetContainer = _.cumulativeOffset(container)
cumulativeOffsetElement = _.cumulativeOffset(element)
if (x) {
targetX =
cumulativeOffsetElement.left - cumulativeOffsetContainer.left + offset
diffX = targetX - initialX
}
if (y) {
targetY =
cumulativeOffsetElement.top - cumulativeOffsetContainer.top + offset
diffY = targetY - initialY
}
Copy link
Owner

Choose a reason for hiding this comment

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

I would refactor this since calculations are same as

vue-scrollto/src/scrollTo.js

Lines 196 to 207 in eb11b5f

initialY = scrollTop(container)
targetY =
cumulativeOffsetElement.top - cumulativeOffsetContainer.top + offset
initialX = scrollLeft(container)
targetX =
cumulativeOffsetElement.left - cumulativeOffsetContainer.left + offset
abort = false
diffY = targetY - initialY
diffX = targetX - initialX

Refactoring both places to use a function would likely be a cleaner approach - since if we ever need to change how the target is calculated, it needs to be done in a single spot.

Something like recalculateTargets, or something better perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do it or you will?

Copy link
Owner

Choose a reason for hiding this comment

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

I probably won't be able to for at least a few weeks, so if you are able to do it, that would be perfect!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

timeElapsed = timestamp - timeStart

progress = Math.min(timeElapsed / duration, 1)
Expand Down Expand Up @@ -143,7 +166,12 @@ export const scroller = () => {
}

container = _.$(options.container || defaults.container)
duration = options.hasOwnProperty('duration') ? options.duration : defaults.duration
duration = options.hasOwnProperty('duration')
? options.duration
: defaults.duration
dynamicPosition = options.hasOwnProperty('dynamicPosition')
? options.dynamicPosition
: defaults.dynamicPosition
easing = options.easing || defaults.easing
offset = options.hasOwnProperty('offset') ? options.offset : defaults.offset
force = options.hasOwnProperty('force')
Expand All @@ -158,8 +186,8 @@ export const scroller = () => {
x = options.x === undefined ? defaults.x : options.x
y = options.y === undefined ? defaults.y : options.y

let cumulativeOffsetContainer = _.cumulativeOffset(container)
let cumulativeOffsetElement = _.cumulativeOffset(element)
cumulativeOffsetContainer = _.cumulativeOffset(container)
cumulativeOffsetElement = _.cumulativeOffset(element)

if (typeof offset === 'function') {
offset = offset(element, container)
Expand Down