Skip to content

Commit ab8a01c

Browse files
posvaposva
committed
feat(scroll): replace selector with el
BREAKING CHANGE: this change follows the RFC at vuejs/rfcs#176: - `selector` is renamed into `el` - `el` also accepts an `Element` - `left` and `top` are passed along `el` instead of inside an object passed as `offset`
1 parent 27a303c commit ab8a01c

File tree

3 files changed

+238
-30
lines changed

3 files changed

+238
-30
lines changed

‎__tests__/scrollBehavior.spec.ts

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
import { JSDOM } from 'jsdom'
2+
import { scrollToPosition } from '../src/scrollBehavior'
3+
import { createDom } from './utils'
4+
import { mockWarn } from 'jest-mock-warn'
5+
6+
describe('scrollBehavior', () => {
7+
mockWarn()
8+
let dom: JSDOM
9+
let scrollTo: jest.SpyInstance
10+
let getElementById: jest.SpyInstance
11+
let querySelector: jest.SpyInstance
12+
13+
beforeAll(() => {
14+
dom = createDom()
15+
scrollTo = jest.spyOn(window, 'scrollTo').mockImplementation(() => {})
16+
getElementById = jest.spyOn(document, 'getElementById')
17+
querySelector = jest.spyOn(document, 'querySelector')
18+
19+
// #text
20+
let el = document.createElement('div')
21+
el.id = 'text'
22+
document.documentElement.appendChild(el)
23+
24+
// [data-scroll]
25+
el = document.createElement('div')
26+
el.setAttribute('data-scroll', 'true')
27+
document.documentElement.appendChild(el)
28+
29+
// #special~characters
30+
el = document.createElement('div')
31+
el.id = 'special~characters'
32+
document.documentElement.appendChild(el)
33+
34+
// #text .container
35+
el = document.createElement('div')
36+
let child = document.createElement('div')
37+
child.classList.add('container')
38+
el.id = 'text'
39+
el.append(child)
40+
document.documentElement.appendChild(el)
41+
42+
// .container #1
43+
el = document.createElement('div')
44+
child = document.createElement('div')
45+
el.classList.add('container')
46+
child.id = '1'
47+
el.append(child)
48+
document.documentElement.appendChild(el)
49+
})
50+
51+
beforeEach(() => {
52+
scrollTo.mockClear()
53+
getElementById.mockClear()
54+
querySelector.mockClear()
55+
__DEV__ = false
56+
})
57+
58+
afterAll(() => {
59+
__DEV__ = true
60+
})
61+
62+
afterAll(() => {
63+
dom.window.close()
64+
scrollTo.mockRestore()
65+
getElementById.mockRestore()
66+
querySelector.mockRestore()
67+
})
68+
69+
describe('left and top', () => {
70+
it('scrolls to a position', () => {
71+
scrollToPosition({ left: 10, top: 100 })
72+
expect(getElementById).not.toHaveBeenCalled()
73+
expect(getElementById).not.toHaveBeenCalled()
74+
expect(scrollTo).toHaveBeenCalledWith({
75+
left: 10,
76+
top: 100,
77+
behavior: undefined,
78+
})
79+
})
80+
81+
it('scrolls to a partial position top', () => {
82+
scrollToPosition({ top: 10 })
83+
expect(getElementById).not.toHaveBeenCalled()
84+
expect(getElementById).not.toHaveBeenCalled()
85+
expect(scrollTo).toHaveBeenCalledWith({
86+
top: 10,
87+
behavior: undefined,
88+
})
89+
})
90+
91+
it('scrolls to a partial position left', () => {
92+
scrollToPosition({ left: 10 })
93+
expect(getElementById).not.toHaveBeenCalled()
94+
expect(getElementById).not.toHaveBeenCalled()
95+
expect(scrollTo).toHaveBeenCalledWith({
96+
left: 10,
97+
behavior: undefined,
98+
})
99+
})
100+
})
101+
102+
describe('el option', () => {
103+
it('scrolls to an id', () => {
104+
scrollToPosition({ el: '#text' })
105+
expect(getElementById).toHaveBeenCalledWith('text')
106+
expect(querySelector).not.toHaveBeenCalled()
107+
expect(scrollTo).toHaveBeenCalledWith({
108+
left: 0,
109+
top: 0,
110+
behavior: undefined,
111+
})
112+
})
113+
114+
it('scrolls to an element using querySelector', () => {
115+
scrollToPosition({ el: '[data-scroll=true]' })
116+
expect(querySelector).toHaveBeenCalledWith('[data-scroll=true]')
117+
expect(getElementById).not.toHaveBeenCalled()
118+
expect(scrollTo).toHaveBeenCalledWith({
119+
left: 0,
120+
top: 0,
121+
behavior: undefined,
122+
})
123+
})
124+
125+
it('scrolls to an id with special characters', () => {
126+
scrollToPosition({ el: '#special~characters' })
127+
expect(getElementById).toHaveBeenCalledWith('special~characters')
128+
expect(querySelector).not.toHaveBeenCalled()
129+
expect(scrollTo).toHaveBeenCalledWith({
130+
left: 0,
131+
top: 0,
132+
behavior: undefined,
133+
})
134+
})
135+
136+
it('scrolls to an id with special characters', () => {
137+
scrollToPosition({ el: '#special~characters' })
138+
expect(getElementById).toHaveBeenCalledWith('special~characters')
139+
expect(querySelector).not.toHaveBeenCalled()
140+
expect(scrollTo).toHaveBeenCalledWith({
141+
left: 0,
142+
top: 0,
143+
behavior: undefined,
144+
})
145+
})
146+
147+
it('accepts a raw element', () => {
148+
scrollToPosition({ el: document.getElementById('special~characters')! })
149+
expect(getElementById).toHaveBeenCalledWith('special~characters')
150+
expect(querySelector).not.toHaveBeenCalled()
151+
expect(scrollTo).toHaveBeenCalledWith({
152+
left: 0,
153+
top: 0,
154+
behavior: undefined,
155+
})
156+
})
157+
158+
describe('warnings', () => {
159+
beforeEach(() => {
160+
__DEV__ = true
161+
})
162+
163+
it('warns if element cannot be found with id', () => {
164+
scrollToPosition({ el: '#not-found' })
165+
expect(
166+
`Couldn't find element using selector "#not-found"`
167+
).toHaveBeenWarned()
168+
})
169+
170+
it('warns if element cannot be found with selector', () => {
171+
scrollToPosition({ el: '.not-found' })
172+
expect(
173+
`Couldn't find element using selector ".not-found"`
174+
).toHaveBeenWarned()
175+
})
176+
177+
it('warns if element cannot be found with id but can with selector', () => {
178+
scrollToPosition({ el: '#text .container' })
179+
expect(
180+
`selector "#text .container" should be passed as "el: document.querySelector('#text .container')"`
181+
).toHaveBeenWarned()
182+
})
183+
184+
it('warns if element cannot be found with id but can with selector', () => {
185+
scrollToPosition({ el: '#text .container' })
186+
expect(
187+
`selector "#text .container" should be passed as "el: document.querySelector('#text .container')"`
188+
).toHaveBeenWarned()
189+
})
190+
191+
it('warns if querySelector throws', () => {
192+
scrollToPosition({ el: '.container #1' })
193+
expect(`selector ".container #1" is invalid`).toHaveBeenWarned()
194+
})
195+
})
196+
})
197+
})

‎e2e/scroll-behavior/index.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,15 @@ const scrollBehavior: ScrollBehavior = async function (
4040

4141
// scroll to anchor by returning the selector
4242
if (to.hash) {
43-
position = { selector: decodeURI(to.hash), offset: { behavior } }
43+
position = { el: decodeURI(to.hash), behavior }
4444

4545
// specify offset of the element
4646
if (to.hash === '#anchor2') {
47-
position.offset = { top: 100, behavior }
47+
position.top = 100
48+
position.behavior = behavior
4849
}
4950

50-
if (document.querySelector(position.selector)) {
51-
return position
52-
}
53-
54-
// if the returned position is falsy or an empty object,
55-
// will retain current scroll position.
56-
return false
51+
return position
5752
}
5853

5954
// check if any matched route config has meta that requires scrolling to top
@@ -86,7 +81,7 @@ const app = createApp({
8681
setup() {
8782
return {
8883
smoothScroll,
89-
hashWithNumber: { path: '/bar', hash: '#\\31 number' },
84+
hashWithNumber: { path: '/bar', hash: '#1number' },
9085
flushWaiter: scrollWaiter.flush,
9186
setupWaiter: scrollWaiter.add,
9287
}

‎src/scrollBehavior.ts

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export type _ScrollPositionNormalized = {
3030
top: number
3131
}
3232

33-
export interface ScrollPositionElement {
33+
export interface ScrollPositionElement extends ScrollToOptions {
3434
/**
3535
* A valid CSS selector. Note some characters must be escaped in id selectors (https://mathiasbynens.be/notes/css-escapes).
3636
* @example
@@ -43,11 +43,7 @@ export interface ScrollPositionElement {
4343
* - `#marker.with.dot`: selects `class="with dot" id="marker"`, not `id="marker.with.dot"`
4444
*
4545
*/
46-
selector: string
47-
/**
48-
* Relative offset to the `selector` in {@link ScrollPositionCoordinates}
49-
*/
50-
offset?: ScrollPositionCoordinates
46+
el: string | Element
5147
}
5248

5349
export type ScrollPosition = ScrollPositionCoordinates | ScrollPositionElement
@@ -85,7 +81,10 @@ export const computeScrollPosition = () =>
8581
export function scrollToPosition(position: ScrollPosition): void {
8682
let scrollToOptions: ScrollPositionCoordinates
8783

88-
if ('selector' in position) {
84+
if ('el' in position) {
85+
let positionEl = position.el
86+
const isIdSelector =
87+
typeof positionEl === 'string' && positionEl.startsWith('#')
8988
/**
9089
* `id`s can accept pretty much any characters, including CSS combinators
9190
* like `>` or `~`. It's still possible to retrieve elements using
@@ -107,33 +106,50 @@ export function scrollToPosition(position: ScrollPosition): void {
107106
* https://mathiasbynens.be/notes/html5-id-class.
108107
* - Practical example: https://mathiasbynens.be/demo/html5-id
109108
*/
110-
if (__DEV__) {
111-
try {
112-
document.querySelector(position.selector)
113-
} catch {
114-
warn(
115-
`The selector "${position.selector}" is invalid. If you are using an id selector, make sure to escape it. You can find more information about escaping characters in selectors at https://mathiasbynens.be/notes/css-escapes.`
116-
)
109+
if (__DEV__ && typeof position.el === 'string') {
110+
if (!isIdSelector || !document.getElementById(position.el.slice(1))) {
111+
try {
112+
let foundEl = document.querySelector(position.el)
113+
if (isIdSelector && foundEl) {
114+
warn(
115+
`The selector "${position.el}" should be passed as "el: document.querySelector('${position.el}')" because it starts with "#".`
116+
)
117+
// return to avoid other warnings
118+
return
119+
}
120+
} catch {
121+
warn(
122+
`The selector "${position.el}" is invalid. If you are using an id selector, make sure to escape it. You can find more information about escaping characters in selectors at https://mathiasbynens.be/notes/css-escapes or use CSS.escape (https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape).`
123+
)
124+
// return to avoid other warnings
125+
return
126+
}
117127
}
118128
}
119129

120-
const el = document.querySelector(position.selector)
130+
const el =
131+
typeof positionEl === 'string'
132+
? isIdSelector
133+
? document.getElementById(positionEl.slice(1))
134+
: document.querySelector(positionEl)
135+
: positionEl
121136

122137
if (!el) {
123-
__DEV__ &&
124-
warn(`Couldn't find element with selector "${position.selector}"`)
138+
__DEV__ && warn(`Couldn't find element using selector "${position.el}"`)
125139
return
126140
}
127-
scrollToOptions = getElementPosition(el, position.offset || {})
141+
scrollToOptions = getElementPosition(el, position)
128142
} else {
129143
scrollToOptions = position
130144
}
131145

132146
if ('scrollBehavior' in document.documentElement.style)
133147
window.scrollTo(scrollToOptions)
134148
else {
135-
// TODO: pass the current value instead of 0 using computeScroll
136-
window.scrollTo(scrollToOptions.left || 0, scrollToOptions.top || 0)
149+
window.scrollTo(
150+
scrollToOptions.left != null ? scrollToOptions.left : window.pageXOffset,
151+
scrollToOptions.top != null ? scrollToOptions.top : window.pageYOffset
152+
)
137153
}
138154
}
139155

0 commit comments

Comments
 (0)