diff --git a/src/helpers.ts b/src/helpers.ts index 0306c233..3cb5cabd 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -31,3 +31,17 @@ export const scrollIntoCenterIfNeeded = (container: HTMLElement, content: HTMLEl container.scrollTop = scrollTop } } + +/** + * Returns a curried function that returns `true` if `e1` and `e2` overlap. + */ +export const elementOverlaps = (e1: HTMLElement) => (e2: HTMLElement): boolean => { + const e1Rect = e1.getBoundingClientRect() + const e2Rect = e2.getBoundingClientRect() + return !( + e1Rect.right < e2Rect.left || + e1Rect.left > e2Rect.right || + e1Rect.bottom < e2Rect.top || + e1Rect.top > e2Rect.bottom + ) +} diff --git a/src/hoverifier.test.ts b/src/hoverifier.test.ts index dca367ef..5b27587f 100644 --- a/src/hoverifier.test.ts +++ b/src/hoverifier.test.ts @@ -1,10 +1,10 @@ import { Range } from '@sourcegraph/extension-api-types' import { isEqual } from 'lodash' import { EMPTY, NEVER, Observable, of, Subject, Subscription } from 'rxjs' -import { delay, distinctUntilChanged, filter, first, map } from 'rxjs/operators' +import { delay, distinctUntilChanged, filter, first, map, takeWhile } from 'rxjs/operators' import { TestScheduler } from 'rxjs/testing' import { ErrorLike } from './errors' -import { propertyIsDefined } from './helpers' +import { isDefined, propertyIsDefined } from './helpers' import { AdjustmentDirection, createHoverifier, @@ -30,6 +30,13 @@ describe('Hoverifier', () => { testcases = dom.createCodeViews() }) + let subscriptions = new Subscription() + + afterEach(() => { + subscriptions.unsubscribe() + subscriptions = new Subscription() + }) + it('highlights token when hover is fetched (not before)', () => { for (const codeView of testcases) { const scheduler = new TestScheduler((a, b) => chai.assert.deepEqual(a, b)) @@ -322,6 +329,55 @@ describe('Hoverifier', () => { } }) + it('hides the hover overlay when the hovered token intersects with a scrollBoundary', async () => { + const gitHubCodeView = testcases[1] + const hoverifier = createHoverifier({ + closeButtonClicks: NEVER, + hoverOverlayElements: of(null), + hoverOverlayRerenders: EMPTY, + getHover: createStubHoverProvider({ + range: { + start: { line: 4, character: 9 }, + end: { line: 4, character: 9 }, + }, + }), + getActions: createStubActionsProvider(['foo', 'bar']), + pinningEnabled: true, + }) + subscriptions.add(hoverifier) + subscriptions.add( + hoverifier.hoverify({ + dom: gitHubCodeView, + positionEvents: of(gitHubCodeView.codeView).pipe( + findPositionsFromEvents({ domFunctions: gitHubCodeView }) + ), + positionJumps: new Subject(), + resolveContext: () => gitHubCodeView.revSpec, + scrollBoundaries: [gitHubCodeView.codeView.querySelector('.sticky-file-header')!], + }) + ) + + gitHubCodeView.codeView.scrollIntoView() + + // Click https://sourcegraph.sgdev.org/github.com/gorilla/mux@cb4698366aa625048f3b815af6a0dea8aef9280a/-/blob/mux.go#L5:9 + // and wait for the hovered token to be defined. + const hasHoveredToken = hoverifier.hoverStateUpdates + .pipe(takeWhile(({ hoveredTokenElement }) => !isDefined(hoveredTokenElement))) + .toPromise() + dispatchMouseEventAtPositionImpure('click', gitHubCodeView, { + line: 5, + character: 9, + }) + await hasHoveredToken + + // Scroll down: the hover overlay should get hidden. + const hoverIsHidden = hoverifier.hoverStateUpdates + .pipe(takeWhile(({ hoverOverlayProps }) => isDefined(hoverOverlayProps))) + .toPromise() + gitHubCodeView.getCodeElementFromLineNumber(gitHubCodeView.codeView, 2)!.scrollIntoView({ behavior: 'smooth' }) + await hoverIsHidden + }) + describe('pinning', () => { it('unpins upon clicking on a different position', () => { for (const codeView of testcases) { diff --git a/src/hoverifier.ts b/src/hoverifier.ts index 4189931a..219e30d4 100644 --- a/src/hoverifier.ts +++ b/src/hoverifier.ts @@ -1,6 +1,7 @@ import { Position, Range } from '@sourcegraph/extension-api-types' import { isEqual } from 'lodash' import { + animationFrameScheduler, combineLatest, concat, EMPTY, @@ -21,7 +22,10 @@ import { delay, distinctUntilChanged, filter, + first, map, + mapTo, + observeOn, share, switchMap, takeUntil, @@ -29,7 +33,7 @@ import { } from 'rxjs/operators' import { Key } from 'ts-key-enum' import { asError, ErrorLike, isErrorLike } from './errors' -import { scrollIntoCenterIfNeeded } from './helpers' +import { elementOverlaps, scrollIntoCenterIfNeeded } from './helpers' import { calculateOverlayPosition } from './overlay_position' import { DiffPart, PositionEvent, SupportedMouseEvent } from './positions' import { createObservableStateContainer } from './state' @@ -178,6 +182,15 @@ export interface EventOptions { adjustPosition?: PositionAdjuster dom: DOMFunctions codeViewId: symbol + + /** + * An array of elements used to hide the hover overlay if any of them + * overlap with the hovered token. Overlapping is checked in reaction to scroll events. + * + * scrollBoundaries are typically elements with a lower z-index than the hover overlay + * but a higher z-index than the code view, such as a sticky file header. + */ + scrollBoundaries?: HTMLElement[] } /** @@ -607,6 +620,84 @@ export function createHoverifier({ distinctUntilChanged((a, b) => isEqual(a.position, b.position)) ) + /** + * An Observable of scroll events on the document. + */ + const scrollEvents = fromEvent(document, 'scroll').pipe( + observeOn(animationFrameScheduler), + share() + ) + + /** + * Returns the highlighted range for the given hover result and position. + * + * Returns `undefined` if the hover result is not successful. + * + * Uses the range specified by the hover result if present, or `position` oherwise, + * which will be expanded into a full token in getTokenAtPosition(). + */ + const getHighlightedRange = ({ + hoverOrError, + position, + }: { + hoverOrError?: typeof LOADING | (HoverAttachment & D) | ErrorLike | null + position: Position | undefined + }): Range | undefined => { + if (hoverOrError && !isErrorLike(hoverOrError) && hoverOrError !== LOADING) { + if (hoverOrError.range) { + // The result is 0-indexed; the code view is treated as 1-indexed. + return { + start: { + line: hoverOrError.range.start.line + 1, + character: hoverOrError.range.start.character + 1, + }, + end: { + line: hoverOrError.range.end.line + 1, + character: hoverOrError.range.end.character + 1, + }, + } + } + if (position) { + return { start: position, end: position } + } + } + return undefined + } + + /** + * Returns an Observable that emits the hover result immediately, + * and will emit a result resetting the hover when the hoveredTokenElement intersects + * with the scrollBoundaries. + */ + const resetOnBoundaryIntersection = ({ + hoveredTokenElement, + scrollBoundaries, + ...rest + }: Omit, 'mouseIsMoving' | 'hoverOverlayIsFixed'> & + Omit, 'resolveContext' | 'dom'> & { codeView: HTMLElement }): Observable< + Omit, 'mouseIsMoving' | 'hoverOverlayIsFixed'> & { codeView: HTMLElement } + > => { + const result = of({ hoveredTokenElement, ...rest }) + if (!hoveredTokenElement || !scrollBoundaries) { + return result + } + return merge( + result, + scrollEvents.pipe( + filter(() => scrollBoundaries.some(elementOverlaps(hoveredTokenElement))), + first(), + mapTo({ + ...rest, + hoveredTokenElement, + hoverOverlayIsFixed: false, + hoverOrError: undefined, + hoveredToken: undefined, + actionsOrError: undefined, + }) + ) + ) + } + /** * For every position, emits an Observable with new values for the `hoverOrError` state. * This is a higher-order Observable (Observable that emits Observables). @@ -619,6 +710,7 @@ export function createHoverifier({ adjustPosition?: PositionAdjuster codeView: HTMLElement codeViewId: symbol + scrollBoundaries?: HTMLElement[] hoverOrError?: typeof LOADING | (HoverAttachment & D) | ErrorLike | null position?: HoveredToken & C part?: DiffPart @@ -662,7 +754,7 @@ export function createHoverifier({ hoverObservables .pipe( switchMap(hoverObservable => hoverObservable), - switchMap(({ hoverOrError, position, adjustPosition, ...rest }) => { + switchMap(({ hoverOrError, position, adjustPosition, codeView, part, ...rest }) => { let pos = hoverOrError && hoverOrError !== LOADING && @@ -673,7 +765,13 @@ export function createHoverifier({ : position if (!pos) { - return of({ hoverOrError, position: undefined as Position | undefined, ...rest }) + return of({ + hoverOrError, + codeView, + part, + position: undefined as Position | undefined, + ...rest, + }) } // The requested position is is 0-indexed; the code here is currently 1-indexed @@ -683,66 +781,51 @@ export function createHoverifier({ const adjustingPosition = adjustPosition ? from( adjustPosition({ - codeView: rest.codeView, + codeView, direction: AdjustmentDirection.ActualToCodeView, position: { ...pos, - part: rest.part, + part, }, }) ) : of(pos) - return adjustingPosition.pipe(map(position => ({ position, hoverOrError, ...rest }))) + return adjustingPosition.pipe( + map(position => ({ position, hoverOrError, codeView, part, ...rest })) + ) + }), + switchMap(({ scrollBoundaries, hoverOrError, position, codeView, codeViewId, dom, part }) => { + const highlightedRange = getHighlightedRange({ hoverOrError, position }) + const hoveredTokenElement = highlightedRange + ? getTokenAtPosition(codeView, highlightedRange.start, dom, part, tokenize) + : undefined + return resetOnBoundaryIntersection({ + scrollBoundaries, + codeViewId, + codeView, + highlightedRange, + hoverOrError, + hoveredTokenElement, + hoverOverlayPosition: undefined, + }) }) ) - .subscribe(({ hoverOrError, position, codeView, codeViewId, dom, part }) => { - // Update the highlighted token if the hover result is successful. If the hover result specifies a - // range, use that; otherwise use the hover position (which will be expanded into a full token in - // getTokenAtPosition). - let highlightedRange: Range | undefined - if (hoverOrError && !isErrorLike(hoverOrError) && hoverOrError !== LOADING) { - if (hoverOrError.range) { - // The result is 0-indexed; the code view is treated as 1-indexed. - highlightedRange = { - start: { - line: hoverOrError.range.start.line + 1, - character: hoverOrError.range.start.character + 1, - }, - end: { - line: hoverOrError.range.end.line + 1, - character: hoverOrError.range.end.character + 1, - }, - } - } else if (position) { - highlightedRange = { start: position, end: position } - } - } - + .subscribe(({ codeView, highlightedRange, hoveredTokenElement, ...rest }) => { container.update({ - codeViewId, - hoverOrError, highlightedRange, - // Reset the hover position, it's gonna be repositioned after the hover was rendered - hoverOverlayPosition: undefined, + hoveredTokenElement, + ...rest, }) - // Ensure the previously highlighted range is not highlighted and the new highlightedRange (if any) // is highlighted. const currentHighlighted = codeView.querySelector('.selection-highlight') if (currentHighlighted) { currentHighlighted.classList.remove('selection-highlight') } - if (!highlightedRange) { - container.update({ hoveredTokenElement: undefined }) - return - } - const token = getTokenAtPosition(codeView, highlightedRange.start, dom, part, tokenize) - container.update({ hoveredTokenElement: token }) - if (!token) { - return + if (hoveredTokenElement) { + hoveredTokenElement.classList.add('selection-highlight') } - token.classList.add('selection-highlight') }) ) diff --git a/testdata/github/generate.ts b/testdata/github/generate.ts index 5731c8de..51cb13d6 100644 --- a/testdata/github/generate.ts +++ b/testdata/github/generate.ts @@ -19,6 +19,7 @@ export function generateGithubCodeTable(lines: string[]): string {
+
${code}
diff --git a/testdata/github/styles.css b/testdata/github/styles.css index 952e8f87..e89614da 100644 --- a/testdata/github/styles.css +++ b/testdata/github/styles.css @@ -10,6 +10,21 @@ color: #24292e; } +.github-testcase .sticky-file-header { + position: sticky; + z-index: 6; + top: 0px; +} + +.github-testcase .file-header { + height: 60px; + padding: 5px 10px; + background-color: #fafbfc; + border-bottom: 1px solid #e1e4e8; + border-top-left-radius: 2px; + border-top-right-radius: 2px; +} + .github-testcase .container { width: 980px; margin-right: auto;