refactor: more consistent naming in dom testutils and add doc blocks

Author: ijsnow

src/hoverifier.test.ts

@@ -53,7 +53,7 @@ describe('Hoverifier', () => {
                     scrollElement: HTMLElement
                 }>()
 
-                const positionEvents = of(codeView.element).pipe(findPositionsFromEvents(codeView))
+                const positionEvents = of(codeView.codeView).pipe(findPositionsFromEvents(codeView))
 
                 const subscriptions = new Subscription()
 

src/hoverifier.ts

@@ -23,7 +23,7 @@ import { PositionEvent, SupportedMouseEvent } from './positions'
 import { createObservableStateContainer } from './state'
 import {
     convertNode,
-    DOMOptions,
+    DOMFunctions,
     findElementWithOffset,
     getCodeElementsInRange,
     getTokenAtPosition,
@@ -55,7 +55,7 @@ interface HoverifierOptions {
 
     hoverOverlayElements: Observable<HTMLElement | null>
 
-    dom: DOMOptions
+    dom: DOMFunctions
 
     /**
      * Called for programmatic navigation (like `history.push()`)

src/positions.test.ts

@@ -42,7 +42,7 @@ describe('position_listener', () => {
                     },
                 }
 
-                const clickedTokens = of(codeView.element).pipe(
+                const clickedTokens = of(codeView.codeView).pipe(
                     findPositionsFromEvents(codeView),
                     filter(propertyIsDefined('position')),
                     map(({ position: { line, character } }) => ({ line, character }))

src/positions.ts

@@ -1,8 +1,7 @@
 import { fromEvent, merge, Observable } from 'rxjs'
 import { filter, map, switchMap, tap } from 'rxjs/operators'
 import { Position } from 'vscode-languageserver-types'
-import { convertCodeElementIdempotent, DOMOptions, HoveredToken, locateTarget } from './token_position'
-
+import { convertCodeElementIdempotent, DOMFunctions, HoveredToken, locateTarget } from './token_position'
 export type SupportedMouseEvent = 'click' | 'mousemove' | 'mouseover'
 
 export interface PositionEvent {
@@ -25,7 +24,7 @@ export interface PositionEvent {
     codeView: HTMLElement
 }
 
-export const findPositionsFromEvents = (options: DOMOptions) => (
+export const findPositionsFromEvents = (options: DOMFunctions) => (
     elements: Observable<HTMLElement>
 ): Observable<PositionEvent> =>
     merge(

src/testutils/dom.test.ts

@@ -7,16 +7,16 @@ describe('can create dom elements from generated code tables', () => {
     after(dom.cleanup)
 
     it('can create the code view test cases and their helper function work', () => {
-        for (const codeView of dom.createCodeViews()) {
+        for (const codeViewProps of dom.createCodeViews()) {
             const {
-                element,
+                codeView,
                 getCodeElementFromTarget,
                 getCodeElementFromLineNumber,
                 getLineNumberFromCodeElement,
-            } = codeView
+            } = codeViewProps
 
             for (let i = 1; i < 10; i++) {
-                const cellFromLine = getCodeElementFromLineNumber(element, i)
+                const cellFromLine = getCodeElementFromLineNumber(codeView, i)
                 expect(cellFromLine).to.not.equal(null)
                 const cellFromTarget = getCodeElementFromTarget(cellFromLine!)
                 expect(cellFromTarget).to.equal(cellFromLine)

src/testutils/dom.ts

@@ -1,9 +1,9 @@
 import githubCode from '../../testdata/generated/github.html'
 import sourcegraphCode from '../../testdata/generated/sourcegraph.html'
-import { DOMOptions, getTextNodes } from '../token_position'
+import { DOMFunctions } from '../token_position'
 import { TEST_DATA_REVSPEC } from './rev'
 
-const createElementFromString = (html: string): HTMLElement => {
+const createElementFromString = (html: string): HTMLDivElement => {
     const elem = document.createElement('div')
 
     elem.innerHTML = html
@@ -17,43 +17,30 @@ const createElementFromString = (html: string): HTMLElement => {
     return elem
 }
 
-export const getCharacterWidth = (character: string): number =>
-    createElementFromString(character).getBoundingClientRect().width
+/** Gets all of the text nodes within a given node. Used for testing. */
+export const getTextNodes = (node: Node): Node[] => {
+    if (node.childNodes.length === 0 && node.TEXT_NODE === node.nodeType && node.nodeValue) {
+        return [node]
+    }
 
-export const getCharacterWidthInContainer = (container: HTMLElement, character: string, idx: number): number => {
-    const span = document.createElement('span')
-    span.innerHTML = character
-    span.dataset.char = idx + ''
-    span.dataset.charCode = character.charCodeAt(0) + ''
-    span.style.visibility = 'hidden'
-    span.style.cssFloat = 'left'
-    span.style.height = '0'
+    const nodes: Node[] = []
 
-    container.appendChild(span)
-    const width = span.getBoundingClientRect().width
-    container.removeChild(span)
+    for (const child of Array.from(node.childNodes)) {
+        nodes.push(...getTextNodes(child))
+    }
 
-    return width
+    return nodes
 }
 
-const getCharactersInCell = (cell: HTMLElement) =>
-    Array.from(
-        getTextNodes(cell)
-            .map(node => node.nodeValue)
-            .join('')
-    )
-
-export const getNumberOfCharactersFromCell = (cell: HTMLElement): number => getCharactersInCell(cell).length
-export const getWidthOfCharactersFromCell = (cell: HTMLElement): number =>
-    getCharactersInCell(cell)
-        .map((c, i) => getCharacterWidthInContainer(cell, c, i))
-        .reduce((a, b) => a + b, 0)
-
-export interface CodeViewProps extends DOMOptions {
-    element: HTMLElement
-    injectedElement: HTMLElement
+/** The props used for the generated test cases(e.g. GitHub and Sourcegraph flavored dom). */
+export interface CodeViewProps extends DOMFunctions {
+    /** The code view for the given test case(e.g. a <code> element in Sourcegraph and <table> in GitHub) */
+    codeView: HTMLElement
+    /** The container of the code view. (e.g. The scrollable contaienr in Sourcegraph and a parent <div> in GitHub) */
+    container: HTMLElement
+    /** The revision and repository information for the file used in the generated test cases. */
     revSpec: typeof TEST_DATA_REVSPEC
-
+    /** A helper method for adding rows to the test case. */
     insertRow: (text: string) => HTMLElement
 }
 
@@ -62,6 +49,8 @@ export const wrapCharsInSpans = (line: string) =>
         .map((c, j) => `<span data-char="${j}">${c}</span>`)
         .join('')
 
+// BEGIN setup test cases
+
 const getDiffCodePart = (codeElement: HTMLElement): 'new' | 'old' | undefined => {
     switch (codeElement.textContent!.charAt(0)) {
         case '+':
@@ -123,8 +112,8 @@ const createGitHubCodeView = (): CodeViewProps => {
     }
 
     return {
-        injectedElement: codeView,
-        element: codeView,
+        container: codeView,
+        codeView,
 
         revSpec: TEST_DATA_REVSPEC,
         getCodeElementFromTarget,
@@ -204,9 +193,9 @@ const createSourcegraphCodeView = (): CodeViewProps => {
     }
 
     return {
-        injectedElement: codeView,
+        container: codeView,
 
-        element: codeView.querySelector('code')!,
+        codeView: codeView.querySelector('code')!,
         revSpec: TEST_DATA_REVSPEC,
         getCodeElementFromTarget,
         getCodeElementFromLineNumber,
@@ -231,31 +220,51 @@ const createSourcegraphCodeView = (): CodeViewProps => {
     }
 }
 
+// END setup test cases
+
+/**
+ * DOM is a testing utility class that keeps track of all elements a test suite is adding to the DOM
+ * so that we can clean up after the test suite has finished.
+ */
 export class DOM {
+    /** The inserted nodes. We save them so that we can remove them on cleanup. */
     private nodes = new Set<Element>()
 
+    /**
+     * Creates and inserts the generated test cases into the DOM
+     * @returns the CodeViewProps for the test cases added to the DOM.
+     */
     public createCodeViews(): CodeViewProps[] {
         const codeViews: CodeViewProps[] = [createSourcegraphCodeView(), createGitHubCodeView()]
 
-        for (const { injectedElement } of codeViews) {
-            this.insert(injectedElement)
+        for (const { container } of codeViews) {
+            this.insert(container)
         }
 
         return codeViews
     }
 
-    public createElementFromString(html: string): HTMLElement {
+    /**
+     * Creates a div with some arbitrary content.
+     * @param html the content of the element you wish to create.
+     * @returns the created div.
+     */
+    public createElementFromString(html: string): HTMLDivElement {
         const element = createElementFromString(html)
         this.insert(element)
-        return element as HTMLElement
+        return element as HTMLDivElement
     }
 
+    /**
+     * Removes all nodes that were inserted from the DOM. This should be called after a test suite has ran.
+     */
     public cleanup = (): void => {
         for (const node of this.nodes) {
             document.body.removeChild(node)
         }
     }
 
+    /** The funnel for inserting elements into the DOM so that we know to remove it in `cleanup()`. */
     private insert(node: Element): void {
         document.body.appendChild(node)
 

src/testutils/mouse.ts

@@ -31,8 +31,8 @@ const invalidPosition = ({ line, character }: Position, message: string) =>
  * @param codeViewProps the codeViewProps props from the test cases.
  * @param position the position to click.
  */
-export const clickPositionImpure = ({ element, getCodeElementFromLineNumber }: CodeViewProps, position: Position) => {
-    const line = getCodeElementFromLineNumber(element, position.line)
+export const clickPositionImpure = ({ codeView, getCodeElementFromLineNumber }: CodeViewProps, position: Position) => {
+    const line = getCodeElementFromLineNumber(codeView, position.line)
     if (!line) {
         throw new Error(invalidPosition(position, 'Line not found'))
     }
@@ -72,8 +72,8 @@ export const clickPositionImpure = ({ element, getCodeElementFromLineNumber }: C
  * @param codeViewProps the CodeViewProps from the generated test cases
  * @param position the 0-indexed position to click
  */
-export const clickPosition = ({ element, getCodeElementFromLineNumber }: CodeViewProps, position: Position) => {
-    const line = getCodeElementFromLineNumber(element, position.line)
+export const clickPosition = ({ codeView, getCodeElementFromLineNumber }: CodeViewProps, position: Position) => {
+    const line = getCodeElementFromLineNumber(codeView, position.line)
     if (!line) {
         throw new Error(invalidPosition(position, 'Line not found'))
     }

src/token_position.test.ts

@@ -1,5 +1,7 @@
+import { Position } from 'vscode-languageserver-types'
+
 import { CodeViewProps, DOM } from './testutils/dom'
-import { convertNode, findElementWithOffset, getTokenAtPosition, locateTarget } from './token_position'
+import { convertNode, findElementWithOffset, getTokenAtPosition, HoveredToken, locateTarget } from './token_position'
 
 const { expect } = chai
 
@@ -153,9 +155,9 @@ describe('token_positions', () => {
             },
         ]
 
-        for (const { element, ...domOptions } of testcases) {
+        for (const { codeView, ...domOptions } of testcases) {
             for (const { token, position } of tokens) {
-                const found = getTokenAtPosition(element, position, domOptions)
+                const found = getTokenAtPosition(codeView, position, domOptions)
 
                 expect(found).to.not.equal(undefined)
                 expect(found!.textContent).to.equal(token)
@@ -164,22 +166,32 @@ describe('token_positions', () => {
     })
 
     it('locateTarget finds the correct token for a target', () => {
-        const positions = [
-            { position: { line: 24, character: 6 }, token: 'NewRouter' },
-            { position: { line: 7, character: 3 }, token: 'import' },
-            { position: { line: 154, character: 2 }, token: 'if' },
-            { position: { line: 257, character: 5 }, token: '=' },
-            { position: { line: 121, character: 9 }, token: '*' },
-            { position: { line: 128, character: 8 }, token: ':' },
+        const positions: {
+            /** A position within the expected token. */
+            atPosition: Position
+            /** The position that locateTarget found. If it works correctly, it is the position of the first character in the token. */
+            foundPosition: Position
+        }[] = [
+            { atPosition: { line: 24, character: 8 }, foundPosition: { line: 24, character: 6 } }, // NewRouter
+            { atPosition: { line: 7, character: 3 }, foundPosition: { line: 7, character: 1 } }, // import
+            { atPosition: { line: 154, character: 3 }, foundPosition: { line: 154, character: 2 } }, // if
+            { atPosition: { line: 257, character: 5 }, foundPosition: { line: 257, character: 5 } }, // =
+            { atPosition: { line: 121, character: 9 }, foundPosition: { line: 121, character: 9 } }, // *
+            { atPosition: { line: 128, character: 8 }, foundPosition: { line: 128, character: 8 } }, // :
         ]
 
-        for (const { element, ...domOptions } of testcases) {
-            for (const { position } of positions) {
-                const target = getTokenAtPosition(element, position, domOptions)
+        for (const { codeView, ...domOptions } of testcases) {
+            for (const { atPosition, foundPosition } of positions) {
+                const target = getTokenAtPosition(codeView, atPosition, domOptions)
 
                 const found = locateTarget(target!, domOptions)
 
                 expect(found).to.not.equal(undefined)
+
+                const token = found as HoveredToken
+
+                expect(token.line).to.equal(foundPosition.line)
+                expect(token.character).to.equal(foundPosition.character)
             }
         }
     })

src/token_position.ts

@@ -1,10 +1,37 @@
 import { Position } from 'vscode-languageserver-types'
 import { LineOrPositionOrRange } from './url'
 
-export interface DOMOptions {
+/**
+ * A collection of methods needed to tell codeintellify how to look at the DOM. These are required for
+ * ensuring that we don't rely on any sort of specific DOM structure.
+ *
+ *
+ */
+export interface DOMFunctions {
+    /**
+     * Get the element containing the code for a line from an event target.
+     * @param target is the event target.
+     * @returns the element containing the code for a line or null if it can't be found. For example, the second <td> inside a <tr> on Sourcegraph and Github.
+     */
     getCodeElementFromTarget: (target: HTMLElement) => HTMLElement | null
+    /**
+     * Get the element containing the code for a line from a code view given a line number.
+     * @param codeView is the code view itself. For example, the <code> element on Sourcegraph or a <table> on GitHub.
+     * @returns the element containing the code for the given line number or null if it can't be found.
+     */
     getCodeElementFromLineNumber: (codeView: HTMLElement, line: number) => HTMLElement | null
+    /**
+     * Gets the line number for a given element containing code for a line.
+     * @param codeElement is the element containing code for a line. When this function is called,
+     * it will be passed the result of either `getCodeElementFromTarget` or `getCodeElementFromLineNumber`.
+     * @returns the line number.
+     */
     getLineNumberFromCodeElement: (codeElement: HTMLElement) => number
+    /**
+     * Determine whether a code element is from the old or new part of a diff or not part of a diff.
+     * @param codeElement is the element containing a line of code.
+     * @returns whether the line is `'old'`, `'new'` or `undefined` if not part of a diff.
+     */
     getDiffCodePart?: (codeElement: HTMLElement) => 'old' | 'new' | undefined
 }
 
@@ -189,7 +216,7 @@ export interface HoveredToken {
     part?: 'old' | 'new'
 }
 
-interface LocateTargetOptions extends DOMOptions {
+interface LocateTargetOptions extends DOMFunctions {
     ignoreFirstChar?: boolean
 }
 
@@ -254,7 +281,7 @@ export function locateTarget(
     return { line }
 }
 
-interface GetCodeElementsInRangeOptions extends DOMOptions {
+interface GetCodeElementsInRangeOptions extends DOMFunctions {
     position?: LineOrPositionOrRange
 }
 
@@ -291,7 +318,7 @@ export const getCodeElementsInRange = (
 export const getTokenAtPosition = (
     codeView: HTMLElement,
     position: Position,
-    options: DOMOptions
+    options: DOMFunctions
 ): HTMLElement | undefined => {
     const codeCell = options.getCodeElementFromLineNumber(codeView, position.line)
     if (!codeCell) {