diff --git a/src/hoverifier.ts b/src/hoverifier.ts index 4467f504..b3a7f27a 100644 --- a/src/hoverifier.ts +++ b/src/hoverifier.ts @@ -407,7 +407,50 @@ export const createHoverifier = ({ .pipe( // with the latest target that came from either a mouseover, click or location change (whatever was the most recent) withLatestFrom(merge(codeMouseOverTargets, codeClickTargets, jumpTargets)), - map(([{ hoverOverlayElement, relativeElement }, { target }]) => + map( + ([ + { hoverOverlayElement, relativeElement }, + { target, position, codeView, dom, adjustPosition, resolveContext }, + ]) => ({ + hoverOverlayElement, + relativeElement, + target, + position, + codeView, + dom, + adjustPosition, + resolveContext, + }) + ), + switchMap(({ position, codeView, adjustPosition, resolveContext, ...rest }) => { + if (!position || !adjustPosition) { + return of({ position, codeView, ...rest }) + } + + return adjustPosition({ + position: { ...position, ...resolveContext(position) }, + codeView, + direction: AdjustmentDirection.ActualToCodeView, + }).pipe( + map(({ line, character }) => ({ + position: { ...position, line, character }, + codeView, + ...rest, + })) + ) + }), + map(({ target, position, codeView, dom, ...rest }) => { + // We should ensure we have the correct dom element to place the overlay above. + // It is possible that tokens span multiple elements meaning that its possible for the + // hover overlay to be placed in the middle of a token. + const token = position ? getTokenAtPosition(codeView, position, dom, position.part) : target + + return { + target: token || target, + ...rest, + } + }), + map(({ hoverOverlayElement, relativeElement, target }) => calculateOverlayPosition({ relativeElement, target, hoverOverlayElement }) ) ) diff --git a/src/token_position.test.ts b/src/token_position.test.ts index 976273c9..c5039f40 100644 --- a/src/token_position.test.ts +++ b/src/token_position.test.ts @@ -160,13 +160,13 @@ describe('token_positions', () => { }) it('gets the full token, even when it crosses multiple elements', () => { - const codeView = dom.createElementFromString('
Token
') + const codeView = dom.createElementFromString('
Token
') const positions = [ // Test walking to the right - { line: 1, character: 1 }, + { line: 1, character: 2 }, // Test walking to the left - { line: 1, character: 3 }, + { line: 1, character: 4 }, ] for (const position of positions) { @@ -177,6 +177,20 @@ describe('token_positions', () => { chai.expect(token!.textContent).to.equal('Token') } }) + + it("doesn't wrap tokens that span multiple elements more than once", () => { + const codeView = dom.createElementFromString('
Token
') + + const domFn = { + getCodeElementFromLineNumber: (code: HTMLElement) => code.children.item(0) as HTMLElement, + } + const position = { line: 1, character: 2 } + + const token1 = getTokenAtPosition(codeView, position, domFn) + const token2 = getTokenAtPosition(codeView, position, domFn) + + chai.expect(token1).to.equal(token2, 'getTokenAtPosition is wrapping tokens more than once') + }) }) describe('locateTarget()', () => { diff --git a/src/token_position.ts b/src/token_position.ts index d37dddd2..aa544a61 100644 --- a/src/token_position.ts +++ b/src/token_position.ts @@ -310,8 +310,26 @@ export function findElementWithOffset(codeElement: HTMLElement, offset: number): tokenRange.setStartBefore(startNode) tokenRange.setEndAfter(endNode) - // Return the common ancestor as the full token. - return tokenRange.commonAncestorContainer as HTMLElement + // If the text nodes are the same, its safe to return the common ancester which is the container element. + if (startNode === endNode || (tokenRange.commonAncestorContainer as HTMLElement).classList.contains('wrapped')) { + return tokenRange.commonAncestorContainer as HTMLElement + } + + // Otherwise, we can't guarantee that the common ancester container doesn't contain + // whitespace or other characters around it. To solve for this case, we'll just + // surround the contents of the range with a new span. + const wrapper = document.createElement('span') + wrapper.classList.add('wrapped') + + // NOTE: We can't use tokenRange.surroundContents(wrapper) because(from https://developer.mozilla.org/en-US/docs/Web/API/Range/surroundContents): + // + // An exception will be thrown, however, if the Range splits a non-Text node with only one of its + // boundary points. That is, unlike the alternative above, if there are partially selected nodes, + // they will not be cloned and instead the operation will fail. + wrapper.appendChild(tokenRange.extractContents()) + tokenRange.insertNode(wrapper) + + return wrapper } /**