From 057ac550bd57e9e6f752f63afb865279dbdd6879 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Fri, 17 Feb 2023 05:13:28 -0800 Subject: [PATCH] Chore: Renderer: Refactor and test long-press and click handlers (#7774) --- .eslintignore | 2 + .gitignore | 2 + .../MdToHtml/createEventHandlingAttrs.test.ts | 86 ++++++++++++++++++ .../MdToHtml/createEventHandlingAttrs.ts | 88 +++++++++++++++++++ .../renderer/MdToHtml/linkReplacement.test.ts | 21 +++++ packages/renderer/MdToHtml/linkReplacement.ts | 12 ++- packages/renderer/MdToHtml/rules/image.ts | 18 ++-- packages/renderer/jest.config.js | 25 +++--- packages/renderer/package.json | 2 + yarn.lock | 2 + 10 files changed, 227 insertions(+), 31 deletions(-) create mode 100644 packages/renderer/MdToHtml/createEventHandlingAttrs.test.ts create mode 100644 packages/renderer/MdToHtml/createEventHandlingAttrs.ts diff --git a/.eslintignore b/.eslintignore index e0caccc5e..ce28a33e3 100644 --- a/.eslintignore +++ b/.eslintignore @@ -800,6 +800,8 @@ packages/renderer/HtmlToHtml.js packages/renderer/InMemoryCache.js packages/renderer/MarkupToHtml.js packages/renderer/MdToHtml.js +packages/renderer/MdToHtml/createEventHandlingAttrs.js +packages/renderer/MdToHtml/createEventHandlingAttrs.test.js packages/renderer/MdToHtml/linkReplacement.js packages/renderer/MdToHtml/linkReplacement.test.js packages/renderer/MdToHtml/renderMedia.js diff --git a/.gitignore b/.gitignore index 33ccc7515..fb9119921 100644 --- a/.gitignore +++ b/.gitignore @@ -788,6 +788,8 @@ packages/renderer/HtmlToHtml.js packages/renderer/InMemoryCache.js packages/renderer/MarkupToHtml.js packages/renderer/MdToHtml.js +packages/renderer/MdToHtml/createEventHandlingAttrs.js +packages/renderer/MdToHtml/createEventHandlingAttrs.test.js packages/renderer/MdToHtml/linkReplacement.js packages/renderer/MdToHtml/linkReplacement.test.js packages/renderer/MdToHtml/renderMedia.js diff --git a/packages/renderer/MdToHtml/createEventHandlingAttrs.test.ts b/packages/renderer/MdToHtml/createEventHandlingAttrs.test.ts new file mode 100644 index 000000000..1ba9637be --- /dev/null +++ b/packages/renderer/MdToHtml/createEventHandlingAttrs.test.ts @@ -0,0 +1,86 @@ +/** + * @jest-environment jsdom + */ + +import { createEventHandlingListeners, Options } from './createEventHandlingAttrs'; +import { describe, beforeAll, it, jest, expect } from '@jest/globals'; + +describe('createEventHandlingAttrs', () => { + let lastMessage: string|undefined = undefined; + const postMessageFn = (message: string) => { + lastMessage = message; + }; + + beforeAll(() => { + lastMessage = undefined; + + jest.useFakeTimers(); + }); + + it('should not create listeners to handle long-press when long press is disabled', () => { + const options: Options = { + enableLongPress: false, + postMessageSyntax: 'postMessageFn', + }; + const listeners = createEventHandlingListeners('someresourceid', options, 'postMessage("click")'); + + // Should not add touchstart/mouseenter/leave listeners when not long-pressing. + expect(listeners.onmouseenter).toBe(''); + expect(listeners.onmouseleave).toBe(''); + expect(listeners.ontouchstart).toBe(''); + expect(listeners.ontouchmove).toBe(''); + expect(listeners.ontouchend).toBe(''); + expect(listeners.ontouchcancel).toBe(''); + }); + + it('should create click listener for given click action', () => { + const options: Options = { + enableLongPress: false, + postMessageSyntax: 'postMessageFn', + }; + const clickAction = 'postMessageFn("click")'; + const listeners = createEventHandlingListeners('someresourceid', options, clickAction); + expect(listeners.onclick).toContain(clickAction); + + postMessageFn('test'); + eval(listeners.onclick); + expect(lastMessage).toBe('click'); + }); + + it('should create ontouch listeners for long press', () => { + const options: Options = { + enableLongPress: true, + postMessageSyntax: 'postMessageFn', + }; + const clickAction: null|string = null; + const listeners = createEventHandlingListeners('resourceidhere', options, clickAction); + + expect(listeners.onclick).toBe(''); + expect(listeners.ontouchstart).not.toBe(''); + + // Clear lastMessage + postMessageFn('test'); + + eval(listeners.ontouchstart); + jest.advanceTimersByTime(1000 * 4); + + expect(lastMessage).toBe('longclick:resourceidhere'); + }); + + it('motion during a long press should cancel the timeout', () => { + const options: Options = { + enableLongPress: true, + postMessageSyntax: 'postMessageFn', + }; + const listeners = createEventHandlingListeners('id', options, null); + + lastMessage = ''; + eval(listeners.ontouchstart); + jest.advanceTimersByTime(100); + eval(listeners.ontouchmove); + jest.advanceTimersByTime(1000 * 100); + + // Message handler should not have been called. + expect(lastMessage).toBe(''); + }); +}); diff --git a/packages/renderer/MdToHtml/createEventHandlingAttrs.ts b/packages/renderer/MdToHtml/createEventHandlingAttrs.ts new file mode 100644 index 000000000..15d10d82f --- /dev/null +++ b/packages/renderer/MdToHtml/createEventHandlingAttrs.ts @@ -0,0 +1,88 @@ + + +import utils from '../utils'; + + +export interface Options { + enableLongPress: boolean; + postMessageSyntax: string; +} + +// longPressTouchStart and clearLongPressTimeout are turned into strings before being called. +// Thus, they should not reference any other non-builtin functions. +const longPressTouchStartFnString = `(onLongPress, longPressDelay) => { + // if touchTimeout is set when ontouchstart is called it means the user has already touched + // the screen once and this is the 2nd touch in this case we assume the user is trying + // to zoom and we don't want to show the menu + if (!!window.touchTimeout) { + clearTimeout(window.touchTimeout); + window.touchTimeout = null; + } else { + window.touchTimeout = setTimeout(() => { + window.touchTimeout = null; + onLongPress(); + }, longPressDelay); + } +}`; + +const clearLongPressTimeoutFnString = `() => { + if (window.touchTimeout) { + clearTimeout(window.touchTimeout); + window.touchTimeout = null; + } +}`; + +// Helper for createEventHandlingAttrs. Exported to facilitate testing. +export const createEventHandlingListeners = (resourceId: string, options: Options, onClickAction: string|null) => { + const eventHandlers = { + ontouchstart: '', + ontouchmove: '', + ontouchend: '', + ontouchcancel: '', + onmouseenter: '', + onmouseleave: '', + onclick: '', + }; + + if (options.enableLongPress) { + const longPressHandler = `(() => ${options.postMessageSyntax}('longclick:${resourceId}'))`; + const touchStart = `(${longPressTouchStartFnString})(${longPressHandler}, ${utils.longPressDelay}); `; + + const callClearLongPressTimeout = `(${clearLongPressTimeoutFnString})(); `; + const touchCancel = callClearLongPressTimeout; + const touchEnd = callClearLongPressTimeout; + + eventHandlers.ontouchstart += touchStart; + eventHandlers.ontouchcancel += touchCancel; + eventHandlers.ontouchmove += touchCancel; + eventHandlers.ontouchend += touchEnd; + } + + if (onClickAction) { + eventHandlers.onclick += onClickAction; + } + + return eventHandlers; +}; + +// Adds event-handling (e.g. long press) code to images and links. +// resourceId is the ID of the image resource or link. +const createEventHandlingAttrs = (resourceId: string, options: Options, onClickAction: string|null) => { + const eventHandlers = createEventHandlingListeners(resourceId, options, onClickAction); + + // Build onfoo="listener" strings and add them to the result. + let result = ''; + for (const listenerType in eventHandlers) { + const eventHandlersDict = eventHandlers as Record; + + // Only create code for non-empty listeners. + if (eventHandlersDict[listenerType].length > 0) { + const listener = eventHandlersDict[listenerType].replace(/["]/g, '"'); + result += ` ${listenerType}="${listener}" `; + } + } + + return result; +}; + +export default createEventHandlingAttrs; diff --git a/packages/renderer/MdToHtml/linkReplacement.test.ts b/packages/renderer/MdToHtml/linkReplacement.test.ts index d7db1c691..b86961652 100644 --- a/packages/renderer/MdToHtml/linkReplacement.test.ts +++ b/packages/renderer/MdToHtml/linkReplacement.test.ts @@ -1,4 +1,5 @@ import linkReplacement from './linkReplacement'; +import { describe, test, expect } from '@jest/globals'; describe('linkReplacement', () => { @@ -57,4 +58,24 @@ describe('linkReplacement', () => { expect(r.indexOf(expectedPrefix)).toBe(0); }); + test('should create ontouch listeners to handle longpress', () => { + const resourceId = 'e6afba55bdf74568ac94f8d1e3578d2c'; + + const linkHtml = linkReplacement(`:/${resourceId}`, { + ResourceModel: {}, + resources: { + [resourceId]: { + item: {}, + localState: { + fetch_status: 2, // FETCH_STATUS_DONE + }, + }, + }, + enableLongPress: true, + }).html; + + expect(linkHtml).toContain('ontouchstart'); + expect(linkHtml).toContain('ontouchend'); + expect(linkHtml).toContain('ontouchcancel'); + }); }); diff --git a/packages/renderer/MdToHtml/linkReplacement.ts b/packages/renderer/MdToHtml/linkReplacement.ts index 321967c4a..c5ca84a99 100644 --- a/packages/renderer/MdToHtml/linkReplacement.ts +++ b/packages/renderer/MdToHtml/linkReplacement.ts @@ -1,4 +1,5 @@ import utils, { ItemIdToUrlHandler } from '../utils'; +import createEventHandlingAttrs from './createEventHandlingAttrs'; const Entities = require('html-entities').AllHtmlEntities; const htmlentities = new Entities().encode; const urlUtils = require('../urlUtils.js'); @@ -93,13 +94,10 @@ export default function(href: string, options: Options = null): LinkReplacementR let js = `${options.postMessageSyntax}(${JSON.stringify(href)}, { resourceId: ${JSON.stringify(resourceId)} }); return false;`; if (options.enableLongPress && !!resourceId) { const onClick = `${options.postMessageSyntax}(${JSON.stringify(href)})`; - const onLongClick = `${options.postMessageSyntax}("longclick:${resourceId}")`; - // if t is set when ontouchstart is called it means the user has already touched the screen once and this is the 2nd touch - // in this case we assume the user is trying to zoom and we don't want to show the menu - const touchStart = `if (typeof(t) !== "undefined" && !!t) { clearTimeout(t); t = null; } else { t = setTimeout(() => { t = null; ${onLongClick}; }, ${utils.longPressDelay}); }`; - const cancel = 'if (!!t) {clearTimeout(t); t=null;'; - const touchEnd = `${cancel} ${onClick};}`; - js = `ontouchstart='${touchStart}' ontouchend='${touchEnd}' ontouchcancel='${cancel} ontouchmove="${cancel}'`; + js = createEventHandlingAttrs(resourceId, { + enableLongPress: options.enableLongPress ?? false, + postMessageSyntax: options.postMessageSyntax ?? 'void', + }, onClick); } else { js = `onclick='${js}'`; } diff --git a/packages/renderer/MdToHtml/rules/image.ts b/packages/renderer/MdToHtml/rules/image.ts index 6f244f310..5ee6aa74c 100644 --- a/packages/renderer/MdToHtml/rules/image.ts +++ b/packages/renderer/MdToHtml/rules/image.ts @@ -1,6 +1,7 @@ import { RuleOptions } from '../../MdToHtml'; import htmlUtils from '../../htmlUtils'; import utils from '../../utils'; +import createEventHandlingAttrs from '../createEventHandlingAttrs'; function plugin(markdownIt: any, ruleOptions: RuleOptions) { const defaultRender = markdownIt.renderer.rules.image; @@ -17,19 +18,14 @@ function plugin(markdownIt: any, ruleOptions: RuleOptions) { const r = utils.imageReplacement(ruleOptions.ResourceModel, src, ruleOptions.resources, ruleOptions.resourceBaseUrl, ruleOptions.itemIdToUrl); if (typeof r === 'string') return r; if (r) { - let js = ''; - if (ruleOptions.enableLongPress) { - const id = r['data-resource-id']; + const id = r['data-resource-id']; - const longPressHandler = `${ruleOptions.postMessageSyntax}('longclick:${id}')`; - // if t is set when ontouchstart is called it means the user has already touched the screen once and this is the 2nd touch - // in this case we assume the user is trying to zoom and we don't want to show the menu - const touchStart = `if (typeof(t) !== 'undefined' && !!t) { clearTimeout(t); t = null; } else { t = setTimeout(() => { t = null; ${longPressHandler}; }, ${utils.longPressDelay}); }`; - const cancel = 'if (!!t) clearTimeout(t); t=null'; + const js = createEventHandlingAttrs(id, { + enableLongPress: ruleOptions.enableLongPress ?? false, + postMessageSyntax: ruleOptions.postMessageSyntax ?? 'void', + }, null); - js = ` ontouchstart="${touchStart}" ontouchend="${cancel}" ontouchcancel="${cancel}" ontouchmove="${cancel}"`; - } - return ``; + return ``; } return defaultRender(tokens, idx, options, env, self); }; diff --git a/packages/renderer/jest.config.js b/packages/renderer/jest.config.js index e047fa0bb..df966ce66 100644 --- a/packages/renderer/jest.config.js +++ b/packages/renderer/jest.config.js @@ -69,14 +69,11 @@ module.exports = { // ], // An array of file extensions your modules use - // moduleFileExtensions: [ - // "js", - // "json", - // "jsx", - // "ts", - // "tsx", - // "node" - // ], + moduleFileExtensions: [ + 'ts', + 'tsx', + 'js', + ], // A map from regular expressions to module names or to arrays of module names that allow to stub out resources with a single module // moduleNameMapper: {}, @@ -145,13 +142,13 @@ module.exports = { // The glob patterns Jest uses to detect test files testMatch: [ - '**/*.test.js', + '**/*.test.ts', ], // An array of regexp pattern strings that are matched against all test paths, matched tests are skipped - // testPathIgnorePatterns: [ - // "/node_modules/" - // ], + testPathIgnorePatterns: [ + '/node_modules/', + ], // The regexp pattern or array of patterns that Jest uses to detect test files // testRegex: [], @@ -169,7 +166,9 @@ module.exports = { // timers: "real", // A map from regular expressions to paths to transformers - // transform: undefined, + transform: { + '\\.(ts|tsx)$': 'ts-jest', + }, // An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation // transformIgnorePatterns: [ diff --git a/packages/renderer/package.json b/packages/renderer/package.json index 7e0f52305..b569de211 100644 --- a/packages/renderer/package.json +++ b/packages/renderer/package.json @@ -21,6 +21,8 @@ "@types/jest": "29.2.6", "@types/node": "18.11.18", "jest": "29.4.1", + "jest-environment-jsdom": "29.4.1", + "ts-jest": "29.0.5", "typescript": "4.9.4" }, "dependencies": { diff --git a/yarn.lock b/yarn.lock index cd6236c6d..0f503e65e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5045,6 +5045,7 @@ __metadata: highlight.js: 11.7.0 html-entities: 1.4.0 jest: 29.4.1 + jest-environment-jsdom: 29.4.1 json-stringify-safe: 5.0.1 katex: 0.13.24 markdown-it: 13.0.1 @@ -5062,6 +5063,7 @@ __metadata: markdown-it-toc-done-right: 4.2.0 md5: 2.3.0 mermaid: 9.2.2 + ts-jest: 29.0.5 typescript: 4.9.4 languageName: unknown linkType: soft