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

Commit 2ed1850

Browse files
authored
fix: get full token from event targets for tooltip positioning (#43)
1 parent 9938078 commit 2ed1850

File tree

3 files changed

+81
-6
lines changed

3 files changed

+81
-6
lines changed

src/hoverifier.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,50 @@ export const createHoverifier = ({
407407
.pipe(
408408
// with the latest target that came from either a mouseover, click or location change (whatever was the most recent)
409409
withLatestFrom(merge(codeMouseOverTargets, codeClickTargets, jumpTargets)),
410-
map(([{ hoverOverlayElement, relativeElement }, { target }]) =>
410+
map(
411+
([
412+
{ hoverOverlayElement, relativeElement },
413+
{ target, position, codeView, dom, adjustPosition, resolveContext },
414+
]) => ({
415+
hoverOverlayElement,
416+
relativeElement,
417+
target,
418+
position,
419+
codeView,
420+
dom,
421+
adjustPosition,
422+
resolveContext,
423+
})
424+
),
425+
switchMap(({ position, codeView, adjustPosition, resolveContext, ...rest }) => {
426+
if (!position || !adjustPosition) {
427+
return of({ position, codeView, ...rest })
428+
}
429+
430+
return adjustPosition({
431+
position: { ...position, ...resolveContext(position) },
432+
codeView,
433+
direction: AdjustmentDirection.ActualToCodeView,
434+
}).pipe(
435+
map(({ line, character }) => ({
436+
position: { ...position, line, character },
437+
codeView,
438+
...rest,
439+
}))
440+
)
441+
}),
442+
map(({ target, position, codeView, dom, ...rest }) => {
443+
// We should ensure we have the correct dom element to place the overlay above.
444+
// It is possible that tokens span multiple elements meaning that its possible for the
445+
// hover overlay to be placed in the middle of a token.
446+
const token = position ? getTokenAtPosition(codeView, position, dom, position.part) : target
447+
448+
return {
449+
target: token || target,
450+
...rest,
451+
}
452+
}),
453+
map(({ hoverOverlayElement, relativeElement, target }) =>
411454
calculateOverlayPosition({ relativeElement, target, hoverOverlayElement })
412455
)
413456
)

src/token_position.test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,13 @@ describe('token_positions', () => {
160160
})
161161

162162
it('gets the full token, even when it crosses multiple elements', () => {
163-
const codeView = dom.createElementFromString('<div>To<span>ken</span></div>')
163+
const codeView = dom.createElementFromString('<div> To<span>ken </span></div>')
164164

165165
const positions = [
166166
// Test walking to the right
167-
{ line: 1, character: 1 },
167+
{ line: 1, character: 2 },
168168
// Test walking to the left
169-
{ line: 1, character: 3 },
169+
{ line: 1, character: 4 },
170170
]
171171

172172
for (const position of positions) {
@@ -177,6 +177,20 @@ describe('token_positions', () => {
177177
chai.expect(token!.textContent).to.equal('Token')
178178
}
179179
})
180+
181+
it("doesn't wrap tokens that span multiple elements more than once", () => {
182+
const codeView = dom.createElementFromString('<div> To<span>ken </span></div>')
183+
184+
const domFn = {
185+
getCodeElementFromLineNumber: (code: HTMLElement) => code.children.item(0) as HTMLElement,
186+
}
187+
const position = { line: 1, character: 2 }
188+
189+
const token1 = getTokenAtPosition(codeView, position, domFn)
190+
const token2 = getTokenAtPosition(codeView, position, domFn)
191+
192+
chai.expect(token1).to.equal(token2, 'getTokenAtPosition is wrapping tokens more than once')
193+
})
180194
})
181195

182196
describe('locateTarget()', () => {

src/token_position.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,26 @@ export function findElementWithOffset(codeElement: HTMLElement, offset: number):
310310
tokenRange.setStartBefore(startNode)
311311
tokenRange.setEndAfter(endNode)
312312

313-
// Return the common ancestor as the full token.
314-
return tokenRange.commonAncestorContainer as HTMLElement
313+
// If the text nodes are the same, its safe to return the common ancester which is the container element.
314+
if (startNode === endNode || (tokenRange.commonAncestorContainer as HTMLElement).classList.contains('wrapped')) {
315+
return tokenRange.commonAncestorContainer as HTMLElement
316+
}
317+
318+
// Otherwise, we can't guarantee that the common ancester container doesn't contain
319+
// whitespace or other characters around it. To solve for this case, we'll just
320+
// surround the contents of the range with a new span.
321+
const wrapper = document.createElement('span')
322+
wrapper.classList.add('wrapped')
323+
324+
// NOTE: We can't use tokenRange.surroundContents(wrapper) because(from https://developer.mozilla.org/en-US/docs/Web/API/Range/surroundContents):
325+
//
326+
// An exception will be thrown, however, if the Range splits a non-Text node with only one of its
327+
// boundary points. That is, unlike the alternative above, if there are partially selected nodes,
328+
// they will not be cloned and instead the operation will fail.
329+
wrapper.appendChild(tokenRange.extractContents())
330+
tokenRange.insertNode(wrapper)
331+
332+
return wrapper
315333
}
316334

317335
/**

0 commit comments

Comments
 (0)