From 9b9e996f089c8d510e661225f6700c32577af3ab Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:13:21 -0500 Subject: [PATCH 1/5] gdd textContent --- .../src/components/ComplexForm.tsx | 5 +++++ .../src/core/signal-generators/dom-gen.ts | 20 +++++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/signals/signals-example/src/components/ComplexForm.tsx b/packages/signals/signals-example/src/components/ComplexForm.tsx index 2bd0e8f33..15861be26 100644 --- a/packages/signals/signals-example/src/components/ComplexForm.tsx +++ b/packages/signals/signals-example/src/components/ComplexForm.tsx @@ -57,6 +57,11 @@ const ComplexForm = () => { + ) } diff --git a/packages/signals/signals/src/core/signal-generators/dom-gen.ts b/packages/signals/signals/src/core/signal-generators/dom-gen.ts index 0057b93b4..6a9483786 100644 --- a/packages/signals/signals/src/core/signal-generators/dom-gen.ts +++ b/packages/signals/signals/src/core/signal-generators/dom-gen.ts @@ -32,11 +32,11 @@ const parseElement = (el: HTMLElement) => { labels: parseLabels((el as HTMLInputElement).labels), name: (el as HTMLInputElement).name, nodeName: el.nodeName, - nodeValue: el.nodeValue, tagName: el.tagName, title: el.title, type: (el as HTMLInputElement).type, value: (el as HTMLInputElement).value, + textContent: el.textContent, } if (el instanceof HTMLSelectElement) { @@ -81,12 +81,14 @@ export class ClickSignalsGenerator implements SignalGenerator { register(emitter: SignalEmitter) { const handleClick = (ev: MouseEvent) => { - const target = (ev.target as HTMLElement) ?? {} - if (this.isClickableElement(target)) { + const target = ev.target as HTMLElement | null + if (!target) return + const el = this.getClosestClickableElement(target) + if (el) { emitter.emit( createInteractionSignal({ eventType: 'click', - target: parseElement(target), + target: parseElement(el), }) ) } @@ -95,12 +97,9 @@ export class ClickSignalsGenerator implements SignalGenerator { return () => document.removeEventListener('click', handleClick) } - private isClickableElement(el: HTMLElement): boolean { - return ( - el instanceof HTMLAnchorElement || - el instanceof HTMLButtonElement || - ['button', 'link'].includes(el.getAttribute('role') ?? '') - ) + private getClosestClickableElement(el: HTMLElement): HTMLElement | null { + // if you click on a nested element, we want to get the closest clickable ancestor. Useful for things like buttons with nested text or images + return el.closest('button, a, [role="button"], [role="link"]') } } @@ -190,4 +189,5 @@ export const domGenerators = [ FormSubmitGenerator, OnChangeGenerator, OnNavigationEventGenerator, + PageScrollGenerator, ] From 785c62ffa94daeaf74b9c8023475b2abd50e9cef Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:23:44 -0500 Subject: [PATCH 2/5] fix --- .../src/tests/signals-vanilla/basic.test.ts | 1 - packages/signals/signals/src/core/signal-generators/dom-gen.ts | 1 - packages/signals/signals/src/types/signals.ts | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts index 42788567e..1db8c8b8c 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts @@ -123,7 +123,6 @@ test('interaction signals', async () => { labels: [], name: '', nodeName: 'BUTTON', - nodeValue: null, tagName: 'BUTTON', title: '', type: 'submit', diff --git a/packages/signals/signals/src/core/signal-generators/dom-gen.ts b/packages/signals/signals/src/core/signal-generators/dom-gen.ts index 6a9483786..c78b3d1ea 100644 --- a/packages/signals/signals/src/core/signal-generators/dom-gen.ts +++ b/packages/signals/signals/src/core/signal-generators/dom-gen.ts @@ -189,5 +189,4 @@ export const domGenerators = [ FormSubmitGenerator, OnChangeGenerator, OnNavigationEventGenerator, - PageScrollGenerator, ] diff --git a/packages/signals/signals/src/types/signals.ts b/packages/signals/signals/src/types/signals.ts index 2ac1b5e41..4a6d8921b 100644 --- a/packages/signals/signals/src/types/signals.ts +++ b/packages/signals/signals/src/types/signals.ts @@ -18,7 +18,6 @@ export type InteractionData = ClickData | SubmitData | ChangeData interface SerializedTarget { // nodeName: Node['nodeName'] // textContent: Node['textContent'] - // nodeValue: Node['nodeValue'] // nodeType: Node['nodeType'] [key: string]: any } From aa53b3727f25175ab0fe5d1e6affeb039be6952d Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:12:16 -0500 Subject: [PATCH 3/5] clean up innerText and textContent --- .../button-click-complex.test.ts | 59 +++++++++++++++++++ .../src/tests/signals-vanilla/index-page.ts | 4 ++ .../src/tests/signals-vanilla/index.html | 8 ++- .../__tests__/dom-gen-helpers.test.ts | 51 ++++++++++++++++ .../src/core/signal-generators/dom-gen.ts | 16 +++-- 5 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts create mode 100644 packages/signals/signals/src/core/signal-generators/__tests__/dom-gen-helpers.test.ts diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts new file mode 100644 index 000000000..914b1ea07 --- /dev/null +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/button-click-complex.test.ts @@ -0,0 +1,59 @@ +import { test, expect } from '@playwright/test' +import type { SegmentEvent } from '@segment/analytics-next' +import { IndexPage } from './index-page' + +const indexPage = new IndexPage() + +const basicEdgeFn = ` + // this is a process signal function + const processSignal = (signal) => {}` + +test.beforeEach(async ({ page }) => { + await indexPage.loadAndWait(page, basicEdgeFn) +}) + +test('button click (complex, with nested items)', async () => { + /** + * Click a button with nested text, ensure that that correct text shows up + */ + await Promise.all([ + indexPage.clickComplexButton(), + indexPage.waitForSignalsApiFlush(), + ]) + + const signalsReqJSON = indexPage.lastSignalsApiReq.postDataJSON() + const interactionSignals = signalsReqJSON.batch.filter( + (el: SegmentEvent) => el.properties!.type === 'interaction' + ) + expect(interactionSignals).toHaveLength(1) + const data = { + eventType: 'click', + target: { + attributes: { + id: 'complex-button', + }, + classList: [], + id: 'complex-button', + labels: [], + name: '', + nodeName: 'BUTTON', + tagName: 'BUTTON', + title: '', + type: 'submit', + innerText: expect.any(String), + textContent: expect.stringContaining( + 'Other Example Button with Nested Text' + ), + value: '', + }, + } + + expect(interactionSignals[0]).toMatchObject({ + event: 'Segment Signal Generated', + type: 'track', + properties: { + type: 'interaction', + data, + }, + }) +}) diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts index c2effcb2e..2b4b67877 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts @@ -36,4 +36,8 @@ export class IndexPage extends BasePage { async clickButton() { return this.page.click('#some-button') } + + async clickComplexButton() { + return this.page.click('#complex-button') + } } diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index.html b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index.html index 8c0635ac1..4d8204c83 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index.html +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/index.html @@ -9,6 +9,13 @@ + +


@@ -19,5 +26,4 @@
- \ No newline at end of file diff --git a/packages/signals/signals/src/core/signal-generators/__tests__/dom-gen-helpers.test.ts b/packages/signals/signals/src/core/signal-generators/__tests__/dom-gen-helpers.test.ts new file mode 100644 index 000000000..a87f58a38 --- /dev/null +++ b/packages/signals/signals/src/core/signal-generators/__tests__/dom-gen-helpers.test.ts @@ -0,0 +1,51 @@ +import { cleanText } from '../dom-gen' + +describe(cleanText, () => { + test('should remove newline characters', () => { + const input = 'Hello\nWorld\n' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should remove tab characters', () => { + const input = 'Hello\tWorld\t' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should replace multiple spaces with a single space', () => { + const input = 'Hello World' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should replace non-breaking spaces with regular spaces', () => { + const input = 'Hello\u00A0World' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should trim leading and trailing spaces', () => { + const input = ' Hello World ' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should handle a combination of special characters', () => { + const input = ' \n\tHello\u00A0 World\n\t ' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) + + test('should return an empty string if input is empty', () => { + const input = '' + const expected = '' + expect(cleanText(input)).toBe(expected) + }) + + test('should return the same string if there are no special characters', () => { + const input = 'Hello World' + const expected = 'Hello World' + expect(cleanText(input)).toBe(expected) + }) +}) diff --git a/packages/signals/signals/src/core/signal-generators/dom-gen.ts b/packages/signals/signals/src/core/signal-generators/dom-gen.ts index c78b3d1ea..06c4ab4d4 100644 --- a/packages/signals/signals/src/core/signal-generators/dom-gen.ts +++ b/packages/signals/signals/src/core/signal-generators/dom-gen.ts @@ -23,6 +23,14 @@ const parseNodeMap = (nodeMap: NamedNodeMap): Record => { }, {} as Record) } +export const cleanText = (str: string): string => { + return str + .replace(/[\r\n\t]+/g, ' ') // Replace newlines and tabs with a space + .replace(/\s\s+/g, ' ') // Replace multiple spaces with a single space + .replace(/\u00A0/g, ' ') // Replace non-breaking spaces with a regular space + .trim() // Trim leading and trailing spaces +} + const parseElement = (el: HTMLElement) => { const base = { // adding a bunch of fields that are not on _all_ elements, but are on enough that it's useful to have them here. @@ -36,7 +44,8 @@ const parseElement = (el: HTMLElement) => { title: el.title, type: (el as HTMLInputElement).type, value: (el as HTMLInputElement).value, - textContent: el.textContent, + textContent: el.textContent && cleanText(el.textContent), + innerText: el.innerText && cleanText(el.innerText), } if (el instanceof HTMLSelectElement) { @@ -67,11 +76,6 @@ const parseElement = (el: HTMLElement) => { src: el.src, volume: el.volume, } - } else if (el instanceof HTMLButtonElement) { - return { - ...base, - innerText: el.innerText, - } } return base } From ab9c43cad9188fdbf2865d628f2e4bca93f063b0 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:16:20 -0500 Subject: [PATCH 4/5] clean up innerText and textContent to make easier to parse --- .changeset/weak-mice-judge.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/weak-mice-judge.md diff --git a/.changeset/weak-mice-judge.md b/.changeset/weak-mice-judge.md new file mode 100644 index 000000000..94f2a395f --- /dev/null +++ b/.changeset/weak-mice-judge.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': patch +--- + +Clean up innerText AND textContent to make easier to parse. From 67b2ebf8f2ce8a0ae409c5c60b2c8addfe89c52c Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:18:42 -0500 Subject: [PATCH 5/5] Update weak-mice-judge.md --- .changeset/weak-mice-judge.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/weak-mice-judge.md b/.changeset/weak-mice-judge.md index 94f2a395f..65ec2e936 100644 --- a/.changeset/weak-mice-judge.md +++ b/.changeset/weak-mice-judge.md @@ -2,4 +2,6 @@ '@segment/analytics-signals': patch --- -Clean up innerText AND textContent to make easier to parse. +- Clean up up innerText AND textContent artifacts to make easier to parse. +- Add textContent field +- Make button Clicks more reliable