Skip to content

Commit

Permalink
Merge pull request #2567 from ag-grid/ag-12776/revert-proxy-dom-preve…
Browse files Browse the repository at this point in the history
…nt-default-click

AG-12776 - Fix chart focus lost on (pointer over) canvas click.
  • Loading branch information
alantreadway authored Sep 20, 2024
2 parents 21ec5d5 + 093d689 commit 3b55104
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,11 @@ export class AriaAnnouncementService {
private readonly domManager: DOMManager,
private readonly layoutManager: LayoutManager
) {
const preventDefault = (e: Event) => e.preventDefault();

this.liveElem = AriaAnnouncementService.createAnnouncer();
this.domManager.addChild('canvas-proxy', 'ag-charts-canvas-proxy', this.liveElem);

// In Chromium, clicking on a <div> starts text highlight, which we don't want.
this.liveElem.parentNode?.addEventListener('mousedown', preventDefault);
this.destroyFns.push(
this.layoutManager.addListener('layout:complete', (ev) => this.onResize(ev)),
() => this.domManager.removeChild('canvas-proxy', 'ag-charts-canvas-proxy'),
() => this.liveElem.parentNode?.removeEventListener('mousedown', preventDefault)
() => this.domManager.removeChild('canvas-proxy', 'ag-charts-canvas-proxy')
);
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 11 additions & 7 deletions packages/ag-charts-website/e2e/toolbar.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test } from '@playwright/test';

import { SELECTORS, gotoExample, setupIntrinsicAssertions, toExamplePageUrl } from './util';
import { SELECTORS, gotoExample, locateCanvas, setupIntrinsicAssertions, toExamplePageUrl } from './util';

test.describe('toolbar', () => {
setupIntrinsicAssertions();
Expand All @@ -10,6 +10,8 @@ test.describe('toolbar', () => {
test('line', async ({ page }) => {
await gotoExample(page, url);

const { bbox } = await locateCanvas(page);

await page.locator('[data-toolbar-group="annotations"][data-toolbar-id="line-menu"]').click();
await expect(page).toHaveScreenshot('line-1-popover.png', { animations: 'disabled' });

Expand All @@ -24,20 +26,22 @@ test.describe('toolbar', () => {
await page.click(SELECTORS.canvas, { position: { x: 200, y: 200 } });
await expect(page).toHaveScreenshot('line-4-complete.png', { animations: 'disabled' });

await page.click(SELECTORS.canvas, { position: { x: 300, y: 300 } });
// Click like a human, on the page, not a very specific DOM element.
await page.mouse.click(bbox.x + 300, bbox.y + 300);

await page.locator(SELECTORS.canvas).press('ControlOrMeta+z');
await page.keyboard.press('ControlOrMeta+z');
await expect(page).toHaveScreenshot('line-5-undo.png', { animations: 'disabled' });

await page.locator(SELECTORS.canvas).press('ControlOrMeta+y');
await page.keyboard.press('ControlOrMeta+y');
await expect(page).toHaveScreenshot('line-6-redo.png', { animations: 'disabled' });

await page.click(SELECTORS.canvas, { position: { x: 150, y: 150 } });
// Click like a human, on the page, not a very specific DOM element.
await page.mouse.click(bbox.x + 150, bbox.y + 150);

await page.locator(SELECTORS.canvas).press('ControlOrMeta+c');
await page.keyboard.press('ControlOrMeta+c');
await expect(page).toHaveScreenshot('line-7-copy.png', { animations: 'disabled' });

await page.locator(SELECTORS.canvas).press('ControlOrMeta+v');
await page.keyboard.press('ControlOrMeta+v');
await expect(page).toHaveScreenshot('line-8-paste.png', { animations: 'disabled' });
});

Expand Down
6 changes: 5 additions & 1 deletion packages/ag-charts-website/e2e/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,14 @@ export async function locateCanvas(page: Page) {
const canvas = await page.locator(SELECTORS.canvas);
const width = Number(await canvasElem.getAttribute('width'));
const height = Number(await canvasElem.getAttribute('height'));
const bbox = await canvas.boundingBox();

if ([width, height].some((n) => [-Infinity, 0, Infinity].includes(n) || isNaN(n))) {
throw new Error(`Invalid canvasDims: { width: ${width}, height: ${height} }`);
}
if (bbox == null) {
throw new Error(`Invalid canvas bbox!`);
}

return { canvas, width, height };
return { canvas, width, height, bbox };
}

0 comments on commit 3b55104

Please sign in to comment.