Skip to content

Commit

Permalink
feat(scroll): handle id selectors starting with a number
Browse files Browse the repository at this point in the history
Closes #2163
  • Loading branch information
posva committed Jul 8, 2019
1 parent 8df4123 commit 799ceca
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
4 changes: 3 additions & 1 deletion examples/scroll-behavior/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const scrollBehavior = function (to, from, savedPosition) {
position.offset = { y: 100 }
}

if (document.querySelector(to.hash)) {
// bypass #1number check
if (/^#\d/.test(to.hash) || document.querySelector(to.hash)) {
return position
}

Expand Down Expand Up @@ -87,6 +88,7 @@ new Vue({
<li><router-link to="/bar">/bar</router-link></li>
<li><router-link to="/bar#anchor">/bar#anchor</router-link></li>
<li><router-link to="/bar#anchor2">/bar#anchor2</router-link></li>
<li><router-link to="/bar#1number">/bar#1number</router-link></li>
</ul>
<transition name="fade" mode="out-in" @after-leave="afterLeave">
<router-view class="view"></router-view>
Expand Down
37 changes: 27 additions & 10 deletions src/util/scroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,27 @@ export function handleScroll (
// wait until re-render finishes before scrolling
router.app.$nextTick(() => {
const position = getScrollPosition()
const shouldScroll = behavior.call(router, to, from, isPop ? position : null)
const shouldScroll = behavior.call(
router,
to,
from,
isPop ? position : null
)

if (!shouldScroll) {
return
}

if (typeof shouldScroll.then === 'function') {
shouldScroll.then(shouldScroll => {
scrollToPosition((shouldScroll: any), position)
}).catch(err => {
if (process.env.NODE_ENV !== 'production') {
assert(false, err.toString())
}
})
shouldScroll
.then(shouldScroll => {
scrollToPosition((shouldScroll: any), position)
})
.catch(err => {
if (process.env.NODE_ENV !== 'production') {
assert(false, err.toString())
}
})
} else {
scrollToPosition(shouldScroll, position)
}
Expand Down Expand Up @@ -114,12 +121,22 @@ function isNumber (v: any): boolean {
return typeof v === 'number'
}

const hashStartsWithNumberRE = /^#\d/

function scrollToPosition (shouldScroll, position) {
const isObject = typeof shouldScroll === 'object'
if (isObject && typeof shouldScroll.selector === 'string') {
const el = document.querySelector(shouldScroll.selector)
// getElementById would still fail if the selector contains a more complicated query like #main[data-attr]
// but at the same time, it doesn't make much sense to select an element with an id and an extra selector
const el = hashStartsWithNumberRE.test(shouldScroll.selector) // $flow-disable-line
? document.getElementById(shouldScroll.selector.slice(1)) // $flow-disable-line
: document.querySelector(shouldScroll.selector)

if (el) {
let offset = shouldScroll.offset && typeof shouldScroll.offset === 'object' ? shouldScroll.offset : {}
let offset =
shouldScroll.offset && typeof shouldScroll.offset === 'object'
? shouldScroll.offset
: {}
offset = normalizeOffset(offset)
position = getElementPosition(el, offset)
} else if (isValidPosition(shouldScroll)) {
Expand Down
12 changes: 11 additions & 1 deletion test/e2e/specs/scroll-behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = {
browser
.url('http://localhost:8080/scroll-behavior/')
.waitForElementVisible('#app', 1000)
.assert.count('li a', 5)
.assert.count('li a', 6)
.assert.containsText('.view', 'home')

.execute(function () {
Expand Down Expand Up @@ -117,6 +117,16 @@ module.exports = {
null,
'scroll to anchor with offset'
)
.execute(function () {
document.querySelector('li:nth-child(6) a').click()
})
.assert.evaluate(
function () {
return document.getElementById('1number').getBoundingClientRect().top < 1
},
null,
'scroll to anchor that starts with number'
)
.end()
}
}

0 comments on commit 799ceca

Please sign in to comment.