feat: hold down ctrl/meta to go-to-def on clicks

src/hoverifier.test.ts

@@ -40,7 +40,7 @@ describe('Hoverifier', () => {
             scheduler.run(({ cold, expectObservable }) => {
                 const hoverifier = createHoverifier({
                     closeButtonClicks: new Observable<MouseEvent>(),
-                    goToDefinitionClicks: new Observable<MouseEvent>(),
+                    goToDefinitionClicks: new Subject<MouseEvent>(),
                     hoverOverlayElements: of(null),
                     hoverOverlayRerenders: EMPTY,
                     fetchHover: createStubHoverFetcher(hover, delayTime),
@@ -139,7 +139,7 @@ describe('Hoverifier', () => {
 
                 const hoverifier = createHoverifier({
                     closeButtonClicks: new Observable<MouseEvent>(),
-                    goToDefinitionClicks: new Observable<MouseEvent>(),
+                    goToDefinitionClicks: new Subject<MouseEvent>(),
                     hoverOverlayElements: of(null),
                     hoverOverlayRerenders: EMPTY,
                     fetchHover,

src/hoverifier.ts

@@ -19,7 +19,7 @@ import { isDefined } from './helpers'
 import { overlayUIHasContent, scrollIntoCenterIfNeeded } from './helpers'
 import { HoverOverlayProps, isJumpURL } from './HoverOverlay'
 import { calculateOverlayPosition } from './overlay_position'
-import { DiffPart, PositionEvent, SupportedMouseEvent } from './positions'
+import { DiffPart, PositionEvent, SupportedEvent } from './positions'
 import { createObservableStateContainer } from './state'
 import {
     convertNode,
@@ -53,7 +53,7 @@ export interface HoverifierOptions {
     /**
      * Emit on this Observable when the Go-To-Definition button in the HoverOverlay was clicked
      */
-    goToDefinitionClicks: Observable<MouseEvent>
+    goToDefinitionClicks: Subject<MouseEvent>
 
     /**
      * Emit on this Observable when the close button in the HoverOverlay was clicked
@@ -269,27 +269,41 @@ export const createHoverifier = ({
         selectedPosition: undefined,
     })
 
-    interface MouseEventTrigger extends PositionEvent, EventOptions {}
+    interface EventTrigger extends PositionEvent, EventOptions {}
 
     // These Subjects aggregate all events from all hoverified code views
-    const allPositionsFromEvents = new Subject<MouseEventTrigger>()
+    const allPositionsFromEvents = new Subject<EventTrigger>()
 
-    const isEventType = <T extends SupportedMouseEvent>(type: T) => (
-        event: MouseEventTrigger
-    ): event is MouseEventTrigger & { eventType: T } => event.eventType === type
+    const isEventType = <T extends SupportedEvent>(type: T) => (
+        event: EventTrigger
+    ): event is EventTrigger & { eventType: T } => event.eventType === type
+    const isGoToDefClick = (event: MouseEvent | KeyboardEvent): boolean => event.ctrlKey || event.metaKey
     const allCodeMouseMoves = allPositionsFromEvents.pipe(filter(isEventType('mousemove')))
     const allCodeMouseOvers = allPositionsFromEvents.pipe(filter(isEventType('mouseover')))
     const allCodeClicks = allPositionsFromEvents.pipe(filter(isEventType('click')))
 
+    // keydown and keyup events only cause allPositionsFromEvents to emit when it's a control/command/meta key
+    const allCodeKeyDowns = allPositionsFromEvents.pipe(filter(isEventType('keydown')))
+    const allCodeKeyUps = allPositionsFromEvents.pipe(filter(isEventType('keyup')))
+
     const allPositionJumps = new Subject<PositionJump & EventOptions>()
 
     const subscription = new Subscription()
 
     /**
-     * click events on the code element, ignoring click events caused by the user selecting text.
+     * click events on the code element, ignoring click events caused by the user selecting text and
+     * click events when the ctrl/meta keys are down.
      * Selecting text should not mess with the hover, hover pinning nor the URL.
      */
-    const codeClicksWithoutSelections = allCodeClicks.pipe(filter(() => window.getSelection().toString() === ''))
+    const codeClicksWithoutSelections = allCodeClicks.pipe(
+        filter(() => window.getSelection().toString() === ''),
+        filter(event => !isGoToDefClick(event.event))
+    )
+
+    /**
+     * click events on the code element, when the ctrl/meta keys are down.
+     */
+    const goToDefCodeClicks = allCodeClicks.pipe(filter(event => isGoToDefClick(event.event)))
 
     // Mouse is moving, don't show the tooltip
     subscription.add(
@@ -314,9 +328,10 @@ export const createHoverifier = ({
         })
     )
 
-    const codeMouseOverTargets = allCodeMouseOvers.pipe(
+    const codeMouseOverTargets = merge(allCodeMouseOvers, allCodeKeyDowns, allCodeKeyUps).pipe(
         map(({ event, ...rest }) => ({
             target: event.target as HTMLElement,
+            goToDefHoverHighlight: isGoToDefClick(event),
             ...rest,
         })),
         debounceTime(50),
@@ -347,6 +362,7 @@ export const createHoverifier = ({
         filter(({ event }) => event.currentTarget !== null),
         map(({ event, ...rest }) => ({
             target: event.target as HTMLElement,
+            goToDefHoverHighlight: false,
             ...rest,
         })),
         switchMap(
@@ -391,11 +407,33 @@ export const createHoverifier = ({
                 return undefined
             }
             const part = dom.getDiffCodePart && dom.getDiffCodePart(target)
-            return { ...rest, eventType: 'jump' as 'jump', target, position: { ...position, part }, codeView, dom }
+            return {
+                ...rest,
+                eventType: 'jump' as 'jump',
+                target,
+                position: { ...position, part },
+                codeView,
+                dom,
+                goToDefHoverHighlight: false,
+            }
         }),
         filter(isDefined)
     )
 
+    /**
+     * Ctrl/Cmd-clicks on tokens should act as go to definition clicks
+     */
+    subscription.add(
+        goToDefCodeClicks
+            .pipe(
+                // On go to definition code clicks (i.e., when the user holds down control/command while clicking on a symbol),
+                // we don't want to open in a new window. In this one situation, set the ctrlKey and metaKey values to false
+                // to prevent goToDefinitionClicks subscribers from assuming this behavior.
+                map(e => ({ ...e.event, ctrlKey: false, metaKey: false } as MouseEvent))
+            )
+            .subscribe(event => goToDefinitionClicks.next(event))
+    )
+
     // REPOSITIONING
     // On every componentDidUpdate (after the component was rerendered, e.g. from a hover state update) resposition
     // the tooltip
@@ -499,7 +537,7 @@ export const createHoverifier = ({
                     }).pipe(map(position => ({ position, hoverOrError, ...rest })))
                 })
             )
-            .subscribe(({ hoverOrError, position, codeView, dom, part }) => {
+            .subscribe(({ hoverOrError, position, codeView, dom, part, goToDefHoverHighlight }) => {
                 container.update({
                     hoverOrError,
                     // Reset the hover position, it's gonna be repositioned after the hover was rendered
@@ -508,6 +546,7 @@ export const createHoverifier = ({
                 const currentHighlighted = codeView.querySelector('.selection-highlight')
                 if (currentHighlighted) {
                     currentHighlighted.classList.remove('selection-highlight')
+                    currentHighlighted.classList.remove('selection-go-to-def-highlight')
                 }
                 if (!position) {
                     return
@@ -518,6 +557,9 @@ export const createHoverifier = ({
                     return
                 }
                 token.classList.add('selection-highlight')
+                if (goToDefHoverHighlight) {
+                    token.classList.add('selection-go-to-def-highlight')
+                }
             })
     )
     // Telemetry for hovers
@@ -670,7 +712,7 @@ export const createHoverifier = ({
     )
     subscription.add(
         goToDefinitionClicks.subscribe(event => {
-            container.update({ clickedGoToDefinition: event.ctrlKey || event.metaKey ? 'new-tab' : 'same-tab' })
+            container.update({ clickedGoToDefinition: isGoToDefClick(event) ? 'new-tab' : 'same-tab' })
         })
     )
 

src/positions.ts

@@ -1,19 +1,19 @@
-import { fromEvent, merge, Observable } from 'rxjs'
+import { fromEvent, merge, Observable, pipe } from 'rxjs'
 import { filter, map, switchMap, tap } from 'rxjs/operators'
 import { Position } from 'vscode-languageserver-types'
 import { convertCodeElementIdempotent, DiffPart, DOMFunctions, HoveredToken, locateTarget } from './token_position'
 
-export type SupportedMouseEvent = 'click' | 'mousemove' | 'mouseover'
+export type SupportedEvent = 'click' | 'mousemove' | 'mouseover' | 'keydown' | 'keyup'
 
 export interface PositionEvent {
     /**
      * The event object
      */
-    event: MouseEvent
+    event: MouseEvent | KeyboardEvent
     /**
-     * The type of mouse event that caused this to emit.
+     * The type of event that caused this to emit.
      */
-    eventType: SupportedMouseEvent
+    eventType: SupportedEvent
     /**
      * The position of the token that the event occured at.
      * or undefined when a target was hovered/clicked that does not correspond to a position (e.g. after the end of the line)
@@ -25,6 +25,24 @@ export interface PositionEvent {
     codeView: HTMLElement
 }
 
+/**
+ * The current target element and code view being hovered over.
+ * Used to select the relevant target on KeyboardEvents (which a user will want to apply to the hovered token, but
+ * which actually register on another, unrelated element that has focus).
+ */
+let hoverTarget: HTMLElement | undefined
+let hoverCodeView: HTMLElement | undefined
+
+const findPositionsForKeyboardEvents = pipe(
+    filter((event: { event: KeyboardEvent; eventType: SupportedEvent }) => event.event.currentTarget !== null),
+    map(({ event, ...rest }) => ({
+        event,
+        target: hoverTarget as HTMLElement,
+        codeView: hoverCodeView as HTMLElement,
+        ...rest,
+    }))
+)
+
 export { DOMFunctions, DiffPart }
 export const findPositionsFromEvents = (options: DOMFunctions) => (
     elements: Observable<HTMLElement>
@@ -34,6 +52,10 @@ export const findPositionsFromEvents = (options: DOMFunctions) => (
             switchMap(element => fromEvent<MouseEvent>(element, 'mouseover')),
             map(event => ({ event, eventType: 'mouseover' as 'mouseover' })),
             filter(({ event }) => event.currentTarget !== null),
+            tap(event => {
+                hoverTarget = event.event.target as HTMLElement
+                hoverCodeView = event.event.currentTarget as HTMLElement
+            }),
             map(({ event, ...rest }) => ({
                 event,
                 target: event.target as HTMLElement,
@@ -63,6 +85,16 @@ export const findPositionsFromEvents = (options: DOMFunctions) => (
                 codeView: event.currentTarget as HTMLElement,
                 ...rest,
             }))
+        ),
+        elements.pipe(
+            switchMap(() => fromEvent<KeyboardEvent>(window, 'keydown')),
+            map(event => ({ event, eventType: 'keydown' as 'keydown' })),
+            findPositionsForKeyboardEvents
+        ),
+        elements.pipe(
+            switchMap(() => fromEvent<KeyboardEvent>(window, 'keyup')),
+            map(event => ({ event, eventType: 'keyup' as 'keyup' })),
+            findPositionsForKeyboardEvents
         )
     ).pipe(
         // Find out the position that was hovered over