Skip to content
This repository was archived by the owner on Nov 25, 2021. It is now read-only.

Commit 46ed1a9

Browse files
committed
fix: listen for mousemove events (previously was not listening)
Fixes an issue where the TOOLTIP_DISPLAY_DELAY behavior (not showing the tooltip until the mouse had stopped moving for 100msec) was ineffective because no mousemove events were being received. This was because there was no mousemove event listener. Test plan: 1. Before applying this fix, move your mouse quickly over a token without stopping for 100msec. (You can increase the TOOLTIP_DISPLAY_DELAY constant to make this easier.) Notice that the tooltip appears despite your mouse not ever being stopped for any period of 100msec. 2. Apply this fix. Now try the same. Notice that the tooltip does not appear until your mouse stops moving for 100msec.
1 parent 7b4b707 commit 46ed1a9

File tree

3 files changed

+151
-4
lines changed

3 files changed

+151
-4
lines changed

src/hoverifier.test.ts

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ import {
1010
AdjustmentDirection,
1111
createHoverifier,
1212
LOADER_DELAY,
13+
MOUSEOVER_DELAY,
1314
PositionAdjuster,
1415
TOOLTIP_DISPLAY_DELAY,
1516
} from './hoverifier'
1617
import { HoverOverlayProps } from './HoverOverlay'
17-
import { findPositionsFromEvents } from './positions'
18+
import { findPositionsFromEvents, SupportedMouseEvent } from './positions'
1819
import { CodeViewProps, DOM } from './testutils/dom'
1920
import { createHoverMerged, createStubHoverFetcher, createStubJumpURLFetcher } from './testutils/lsp'
2021
import { dispatchMouseEventAtPositionImpure } from './testutils/mouse'
@@ -112,6 +113,147 @@ describe('Hoverifier', () => {
112113
}
113114
})
114115

116+
it('debounces mousemove events before showing overlay', () => {
117+
for (const codeView of testcases) {
118+
const scheduler = new TestScheduler((a, b) => chai.assert.deepEqual(a, b))
119+
120+
const hover = {}
121+
const defURL = 'def url'
122+
123+
scheduler.run(({ cold, expectObservable }) => {
124+
const hoverifier = createHoverifier({
125+
closeButtonClicks: new Observable<MouseEvent>(),
126+
goToDefinitionClicks: new Observable<MouseEvent>(),
127+
hoverOverlayElements: of(null),
128+
hoverOverlayRerenders: EMPTY,
129+
fetchHover: createStubHoverFetcher(hover),
130+
fetchJumpURL: createStubJumpURLFetcher(defURL),
131+
pushHistory: noop,
132+
getReferencesURL: () => null,
133+
})
134+
135+
const positionJumps = new Subject<{
136+
position: Position
137+
codeView: HTMLElement
138+
scrollElement: HTMLElement
139+
}>()
140+
141+
const positionEvents = of(codeView.codeView).pipe(findPositionsFromEvents(codeView))
142+
143+
const subscriptions = new Subscription()
144+
145+
subscriptions.add(hoverifier)
146+
subscriptions.add(
147+
hoverifier.hoverify({
148+
dom: codeView,
149+
positionEvents,
150+
positionJumps,
151+
resolveContext: () => codeView.revSpec,
152+
})
153+
)
154+
155+
const hoverAndDefinitionUpdates = hoverifier.hoverStateUpdates.pipe(
156+
filter(propertyIsDefined('hoverOverlayProps')),
157+
map(({ hoverOverlayProps }) => !!hoverOverlayProps),
158+
distinctUntilChanged(isEqual)
159+
)
160+
161+
const mousemoveDelay = 25
162+
const outputDiagram = `${TOOLTIP_DISPLAY_DELAY + mousemoveDelay}ms a`
163+
164+
const outputValues: { [key: string]: boolean } = {
165+
a: true,
166+
}
167+
168+
// Mousemove on https://sourcegraph.sgdev.org/github.com/gorilla/mux@cb4698366aa625048f3b815af6a0dea8aef9280a/-/blob/mux.go#L24:6
169+
cold(`a b ${mousemoveDelay - 2}ms c ${TOOLTIP_DISPLAY_DELAY - 1}ms`, {
170+
a: 'mouseover',
171+
b: 'mousemove',
172+
c: 'mousemove',
173+
} as Record<string, SupportedMouseEvent>).subscribe(eventType =>
174+
dispatchMouseEventAtPositionImpure(eventType, codeView, {
175+
line: 24,
176+
character: 6,
177+
})
178+
)
179+
180+
expectObservable(hoverAndDefinitionUpdates).toBe(outputDiagram, outputValues)
181+
})
182+
}
183+
})
184+
185+
it('dedupes mouseover and mousemove event on same token', () => {
186+
for (const codeView of testcases) {
187+
const scheduler = new TestScheduler((a, b) => chai.assert.deepEqual(a, b))
188+
189+
const hover = {}
190+
const defURL = 'def url'
191+
192+
scheduler.run(({ cold, expectObservable }) => {
193+
const hoverifier = createHoverifier({
194+
closeButtonClicks: new Observable<MouseEvent>(),
195+
goToDefinitionClicks: new Observable<MouseEvent>(),
196+
hoverOverlayElements: of(null),
197+
hoverOverlayRerenders: EMPTY,
198+
fetchHover: createStubHoverFetcher(hover),
199+
fetchJumpURL: createStubJumpURLFetcher(defURL),
200+
pushHistory: noop,
201+
getReferencesURL: () => null,
202+
})
203+
204+
const positionJumps = new Subject<{
205+
position: Position
206+
codeView: HTMLElement
207+
scrollElement: HTMLElement
208+
}>()
209+
210+
const positionEvents = of(codeView.codeView).pipe(findPositionsFromEvents(codeView))
211+
212+
const subscriptions = new Subscription()
213+
214+
subscriptions.add(hoverifier)
215+
subscriptions.add(
216+
hoverifier.hoverify({
217+
dom: codeView,
218+
positionEvents,
219+
positionJumps,
220+
resolveContext: () => codeView.revSpec,
221+
})
222+
)
223+
224+
const hoverAndDefinitionUpdates = hoverifier.hoverStateUpdates.pipe(
225+
filter(propertyIsDefined('hoverOverlayProps')),
226+
map(({ hoverOverlayProps }) => !!hoverOverlayProps),
227+
distinctUntilChanged(isEqual)
228+
)
229+
230+
// Add 2 for 1 tick each for "c" and "d" below.
231+
const outputDiagram = `${TOOLTIP_DISPLAY_DELAY + MOUSEOVER_DELAY + 2}ms a`
232+
233+
const outputValues: { [key: string]: boolean } = {
234+
a: true,
235+
}
236+
237+
// Mouse on https://sourcegraph.sgdev.org/github.com/gorilla/mux@cb4698366aa625048f3b815af6a0dea8aef9280a/-/blob/mux.go#L24:6
238+
cold(`a b ${MOUSEOVER_DELAY - 2}ms c d e`, {
239+
a: 'mouseover',
240+
b: 'mousemove',
241+
// Now perform repeated mousemove/mouseover events on the same token.
242+
c: 'mousemove',
243+
d: 'mouseover',
244+
e: 'mousemove',
245+
} as Record<string, SupportedMouseEvent>).subscribe(eventType =>
246+
dispatchMouseEventAtPositionImpure(eventType, codeView, {
247+
line: 24,
248+
character: 6,
249+
})
250+
)
251+
252+
expectObservable(hoverAndDefinitionUpdates).toBe(outputDiagram, outputValues)
253+
})
254+
}
255+
})
256+
115257
/**
116258
* This test ensures that the adjustPosition options is being called in the ways we expect. This test is actually not the best way to ensure the feature
117259
* works as expected. This is a good example of a bad side effect of how the main `hoverifier.ts` file is too tightly integrated with itself. Ideally, I'd be able to assert

src/hoverifier.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,9 @@ export const LOADER_DELAY = 1200
266266
/** The time in ms after the mouse has stopped moving in which to show the tooltip */
267267
export const TOOLTIP_DISPLAY_DELAY = 100
268268

269+
/** The time in ms to debounce mouseover events. */
270+
export const MOUSEOVER_DELAY = 50
271+
269272
/**
270273
* @template C Extra context for the hovered token.
271274
*/
@@ -360,7 +363,7 @@ export function createHoverifier<C extends object>({
360363
target: event.target as HTMLElement,
361364
...rest,
362365
})),
363-
debounceTime(50),
366+
debounceTime(MOUSEOVER_DELAY),
364367
// Do not consider mouseovers while overlay is pinned
365368
filter(() => !container.values.hoverOverlayIsFixed),
366369
switchMap(

src/positions.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@ export const findPositionsFromEvents = (options: DOMFunctions) => (
3131
): Observable<PositionEvent> =>
3232
merge(
3333
from(elements).pipe(
34-
switchMap(element => fromEvent<MouseEvent>(element, 'mouseover')),
35-
map(event => ({ event, eventType: 'mouseover' as 'mouseover' })),
34+
switchMap(element =>
35+
merge(fromEvent<MouseEvent>(element, 'mouseover'), fromEvent<MouseEvent>(element, 'mousemove'))
36+
),
37+
map(event => ({ event, eventType: event.type as 'mouseover' | 'mousemove' })),
3638
filter(({ event }) => event.currentTarget !== null),
3739
map(({ event, ...rest }) => ({
3840
event,

0 commit comments

Comments
 (0)