refactor: getClickableAncestor -> getClickableGroup

src/web/PageEventCollector.ts

@@ -1,8 +1,7 @@
 import { DEFAULT_ATTRIBUTE_LIST } from './attribute';
 import {
-  getClickableAncestor,
   getInputElementValue,
-  getTopmostEditableElement,
+  getMouseEventTarget,
   isVisible,
 } from './element';
 import { buildSelector } from './selector';
@@ -52,7 +51,13 @@ export class PageEventCollector {
     const eventCallback: EventCallback = (window as any).qawElementEvent;
     if (!eventCallback) return;
 
-    const target = event.target as HTMLElement;
+    const isClick = ['click', 'mousedown'].includes(eventName);
+
+    let target = event.target as HTMLElement;
+    if (isClick) {
+      target = getMouseEventTarget(event as MouseEvent);
+    }
+
     const isTargetVisible = isVisible(target, window.getComputedStyle(target));
 
     const elementEvent: types.ElementEvent = {
@@ -61,7 +66,7 @@ export class PageEventCollector {
       page: -1, // set in ContextEventCollector
       selector: buildSelector({
         attributes: this._attributes,
-        isClick: ['click', 'mousedown'].includes(eventName),
+        isClick,
         target,
       }),
       target: nodeToDoc(target),
@@ -81,45 +86,38 @@ export class PageEventCollector {
   }
 
   private collectEvents(): void {
+    //////// MOUSE EVENTS ////////
+
     this.listen('mousedown', (event) => {
       // only the main button (not right clicks/etc)
       // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button
       if (event.button !== 0) return;
 
-      // getClickableAncestor chooses the top most clickable ancestor.
-      // The ancestor is likely a better target than the descendant.
-      // Ex. when you click on the i (button > i) or rect (a > svg > rect)
-      // chances are the ancestor (button, a) is a better target to find.
-      // XXX if anyone runs into issues with this behavior we can allow disabling it from a flag.
-      let target = getClickableAncestor(
-        event.target as HTMLElement,
-        this._attributes,
-      );
-      target = getTopmostEditableElement(target);
-      this.sendEvent('mousedown', { ...event, target });
-    });
-
-    this.listen('change', (event) => {
-      const target = event.target as HTMLInputElement;
-      this.sendEvent('change', event, getInputElementValue(target));
+      this.sendEvent('mousedown', event);
     });
 
     this.listen('click', (event) => {
+      // only the main button (not right clicks/etc)
+      // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button
       if (event.button !== 0) return;
 
-      let target = getClickableAncestor(
-        event.target as HTMLElement,
-        this._attributes,
-      );
-      target = getTopmostEditableElement(target);
-      this.sendEvent('click', { ...event, target });
+      this.sendEvent('click', event);
     });
 
+    //////// INPUT EVENTS ////////
+
     this.listen('input', (event) => {
       const target = event.target as HTMLInputElement;
       this.sendEvent('input', event, getInputElementValue(target));
     });
 
+    this.listen('change', (event) => {
+      const target = event.target as HTMLInputElement;
+      this.sendEvent('change', event, getInputElementValue(target));
+    });
+
+    //////// KEYBOARD EVENTS ////////
+
     this.listen('keydown', (event) => {
       this.sendEvent('keydown', event, event.key);
     });
@@ -128,6 +126,8 @@ export class PageEventCollector {
       this.sendEvent('keyup', event, event.key);
     });
 
+    //////// OTHER EVENTS ////////
+
     this.listen('paste', (event) => {
       if (!event.clipboardData) return;
 

src/web/element.ts

@@ -1,4 +1,3 @@
-import { hasAttribute } from './attribute';
 import { getXpath } from './serialize';
 
 export const isVisible = (
@@ -31,52 +30,38 @@ export const isClickable = (
   return clickable && isVisible(element, computedStyle);
 };
 
-export const getClickableAncestor = (
-  element: HTMLElement,
-  attributes: string[],
-): HTMLElement => {
-  /**
-   * Crawl up until we reach the top "clickable" ancestor.
-   * If the target is the descendant of "a"/"button"/"input" or a clickable element choose it.
-   * If the target has a preferred attribute choose it.
-   * Otherwise choose the original element as the target.
-   */
-  let ancestor = element;
+/**
+ * @summary Sometimes there is a group of elements that together make up what appears
+ *   to be a single button, link, image, etc. Examples: a > span | button > div > span
+ *   For these we want to take into consideration the entire "clickable group" when
+ *   building a good selector. The topmost clickable (a | button | input) should be
+ *   preferred in many cases, but if an inner element has a lower-penalty attribute
+ *   then that should be preferred.
+ *
+ * @return An array of HTMLElement that make up the clickable group. If `element`
+ *   itself is not clickable, the array is empty.
+ */
+export const getClickableGroup = (element: HTMLElement): HTMLElement[] => {
   console.debug('qawolf: get clickable ancestor for', getXpath(element));
 
-  while (ancestor.parentElement) {
-    if (['a', 'button', 'input'].includes(ancestor.tagName.toLowerCase())) {
-      // stop crawling when the ancestor is a good clickable tag
-      console.debug(
-        `qawolf: found clickable ancestor: ${ancestor.tagName}`,
-        getXpath(ancestor),
-      );
-      return ancestor;
-    }
+  const clickableElements = [];
+  let checkElement = element;
 
-    if (hasAttribute(ancestor, attributes)) {
-      // stop crawling when the ancestor has a preferred attribute
-      return ancestor;
-    }
+  while (isClickable(checkElement, window.getComputedStyle(checkElement))) {
+    clickableElements.push(checkElement);
 
-    if (
-      !isClickable(
-        ancestor.parentElement,
-        window.getComputedStyle(ancestor.parentElement),
-      )
-    ) {
-      // stop crawling at the first non-clickable element
-      console.debug('qawolf: found clickable ancestor', getXpath(ancestor));
-      return ancestor;
+    if (['a', 'button', 'input'].includes(checkElement.tagName.toLowerCase())) {
+      // stop crawling when the checkElement is a good clickable tag
+      break;
     }
 
-    ancestor = ancestor.parentElement;
-  }
+    checkElement = checkElement.parentElement;
 
-  // stop crawling at the root
-  console.debug('qawolf: found clickable ancestor', getXpath(ancestor));
+    // stop crawling at the root
+    if (!checkElement) break;
+  }
 
-  return ancestor;
+  return clickableElements;
 };
 
 /**
@@ -110,6 +95,23 @@ export const getTopmostEditableElement = (
   return element;
 };
 
+/**
+ * @summary Returns the best target element for reproducing a mouse event.
+ */
+export const getMouseEventTarget = (event: MouseEvent): HTMLElement => {
+  const originalTarget = event.target as HTMLElement;
+
+  const clickableGroup = getClickableGroup(originalTarget);
+
+  // If originalTarget wasn't part of a clickable group
+  if (clickableGroup.length === 0) {
+    return getTopmostEditableElement(originalTarget);
+  }
+
+  // For now, just return the topmost clickable element in the group
+  return clickableGroup[clickableGroup.length - 1];
+};
+
 /**
  * @summary Returns the current "value" of an element. Pass in an event `target`.
  *   For example, returns the `.value` or the `.innerText` of a content-editable.

src/web/qawolf.ts

@@ -6,8 +6,9 @@ export {
   getCueTypesConfig,
 } from './cues';
 export {
-  getClickableAncestor,
+  getClickableGroup,
   getInputElementValue,
+  getMouseEventTarget,
   getTopmostEditableElement,
   isClickable,
   isVisible,

test/web/element.test.ts

@@ -16,49 +16,53 @@ beforeAll(async () => {
 
 afterAll(() => browser.close());
 
-describe('getClickableAncestor', () => {
+describe('getClickableGroup', () => {
   beforeAll(() => page.goto(`${TEST_URL}buttons`));
 
-  it('chooses the top most clickable ancestor', async () => {
-    const id = await page.evaluate(() => {
+  it('returns a clickable group', async () => {
+    const group = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
       const element = document.querySelector('#nested span') as HTMLElement;
       if (!element) throw new Error('element not found');
 
-      const ancestor = web.getClickableAncestor(element, []);
-      return ancestor.id;
+      return web.getClickableGroup(element).map((el) => el.tagName);
     });
 
-    expect(id).toEqual('nested');
+    expect(group).toMatchInlineSnapshot(`
+      Array [
+        "SPAN",
+        "DIV",
+        "BUTTON",
+      ]
+    `);
   });
 
-  it('chooses the original element when there is no clickable ancestor', async () => {
-    const id = await page.evaluate(() => {
+  it('group has only the original element when there is no clickable ancestor', async () => {
+    const group = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
       const element = document.querySelector('#nested') as HTMLElement;
       if (!element) throw new Error('element not found');
 
-      const ancestor = web.getClickableAncestor(element, []);
-      return ancestor.id;
+      return web.getClickableGroup(element).map((el) => el.tagName);
     });
 
-    expect(id).toEqual('nested');
+    expect(group).toMatchInlineSnapshot(`
+      Array [
+        "BUTTON",
+      ]
+    `);
   });
 
-  it('stops at an ancestor with a preferred attribute', async () => {
-    const attribute = await page.evaluate(() => {
+  it('returns empty array if the element is not clickable', async () => {
+    const groupLength = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
-
-      const element = document.querySelector(
-        '[data-qa="nested-attribute"] span',
-      ) as HTMLElement;
+      const element = document.querySelector('h3') as HTMLElement;
       if (!element) throw new Error('element not found');
 
-      const ancestor = web.getClickableAncestor(element, ['data-qa']);
-      return ancestor.getAttribute('data-qa');
+      return web.getClickableGroup(element).length;
     });
 
-    expect(attribute).toEqual('nested-attribute');
+    expect(groupLength).toBe(0);
   });
 });
 
@@ -68,7 +72,9 @@ describe('getInputElementValue', () => {
 
     const value = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
-      const element = document.querySelector('[data-qa="html-text-input"]') as HTMLInputElement;
+      const element = document.querySelector(
+        '[data-qa="html-text-input"]',
+      ) as HTMLInputElement;
       if (!element) throw new Error('element not found');
 
       element.value = 'I have value';
@@ -84,7 +90,9 @@ describe('getInputElementValue', () => {
 
     const value = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
-      const element = document.querySelector('[data-qa="content-editable"]') as HTMLInputElement;
+      const element = document.querySelector(
+        '[data-qa="content-editable"]',
+      ) as HTMLInputElement;
       if (!element) throw new Error('element not found');
 
       return web.getInputElementValue(element);
@@ -98,7 +106,9 @@ describe('getInputElementValue', () => {
 
     const value = await page.evaluate(() => {
       const web: QAWolfWeb = (window as any).qawolf;
-      const element = document.querySelector('[data-qa="html-text-input-content-editable"]') as HTMLInputElement;
+      const element = document.querySelector(
+        '[data-qa="html-text-input-content-editable"]',
+      ) as HTMLInputElement;
       if (!element) throw new Error('element not found');
 
       element.value = 'I have value';

test/web/selector.test.ts

@@ -38,7 +38,8 @@ describe('buildSelector', () => {
     const builtSelector = await page.evaluate(
       ({ attributes, element, isClick }) => {
         const qawolf: QAWolfWeb = (window as any).qawolf;
-        const target = qawolf.getClickableAncestor(element as HTMLElement, []);
+        let target = qawolf.getClickableGroup(element as HTMLElement).pop();
+        if (!target) target = element as HTMLElement;
 
         qawolf.clearSelectorCache();