Merge pull request #1963 from ag-grid/AG-12042-zoom-to-here

AG-12042 Fix zoom to here going to wrong position

packages/ag-charts-enterprise/src/features/zoom/zoom.ts

@@ -179,7 +179,13 @@ export class Zoom extends _ModuleSupport.BaseModuleInstance implements _ModuleSu
 
  const selectionRect = new ZoomRect();
  this.selector = new ZoomSelector(selectionRect);
- this.contextMenu = new ZoomContextMenu(ctx.contextMenuRegistry, ctx.zoomManager, this.updateZoom.bind(this));
+ this.contextMenu = new ZoomContextMenu(
+ ctx.contextMenuRegistry,
+ ctx.zoomManager,
+ this.getModuleProperties.bind(this),
+ () => this.paddedRect,
+ this.updateZoom.bind(this)
+ );
  this.toolbar = new ZoomToolbar(
  ctx.toolbarManager,
  ctx.zoomManager,
@@ -230,7 +236,7 @@ export class Zoom extends _ModuleSupport.BaseModuleInstance implements _ModuleSu
 
  const zoom = this.getZoom();
  const props = this.getModuleProperties({ enabled });
- this.contextMenu.registerActions(enabled, zoom, props);
+ this.contextMenu.registerActions(enabled, zoom);
  this.onZoomButtonsChange(enabled);
  this.toolbar.toggle(enabled, zoom, props);
  }
@@ -563,7 +569,6 @@ export class Zoom extends _ModuleSupport.BaseModuleInstance implements _ModuleSu
 
  this.seriesRect = rect;
  this.paddedRect = paddedRect;
- this.contextMenu.rect = paddedRect;
  this.shouldFlipXY = shouldFlipXY;
 
  if (!axes) return;
@@ -619,7 +624,7 @@ export class Zoom extends _ModuleSupport.BaseModuleInstance implements _ModuleSu
 
  const zoom = this.getZoom();
  const props = this.getModuleProperties();
- this.contextMenu.toggleActions(zoom, props);
+ this.contextMenu.toggleActions(zoom);
  this.toolbar.toggleButtons(zoom, props);
  }
 

packages/ag-charts-enterprise/src/features/zoom/zoomContextMenu.ts

@@ -19,15 +19,15 @@ const CONTEXT_ZOOM_ACTION_ID = 'zoom-action';
 const CONTEXT_PAN_ACTION_ID = 'pan-action';
 
 export class ZoomContextMenu {
- public rect?: _Scene.BBox;
-
  constructor(
  private readonly contextMenuRegistry: _ModuleSupport.ContextMenuRegistry,
  private readonly zoomManager: _ModuleSupport.ZoomManager,
+ private readonly getModuleProperties: () => ZoomProperties,
+ private readonly getRect: () => _Scene.BBox | undefined,
  private readonly updateZoom: (zoom: DefinedZoomState) => void
  ) {}
 
- public registerActions(enabled: boolean | undefined, zoom: DefinedZoomState, props: ZoomProperties) {
+ public registerActions(enabled: boolean | undefined, zoom: DefinedZoomState) {
  if (!enabled) return;
 
  const { contextMenuRegistry } = this;
@@ -36,22 +36,23 @@ export class ZoomContextMenu {
  id: CONTEXT_ZOOM_ACTION_ID,
  type: 'series',
  label: 'contextMenuZoomToCursor',
- action: (params) => this.onZoomToHere(params, props),
+ action: this.onZoomToHere.bind(this),
  });
  contextMenuRegistry.registerDefaultAction({
  id: CONTEXT_PAN_ACTION_ID,
  type: 'series',
  label: 'contextMenuPanToCursor',
- action: (params) => this.onPanToHere(params, props),
+ action: this.onPanToHere.bind(this),
  });
 
- this.toggleActions(zoom, props);
+ this.toggleActions(zoom);
  }
 
- public toggleActions(zoom: DefinedZoomState, props: ZoomProperties) {
+ public toggleActions(zoom: DefinedZoomState) {
  const { contextMenuRegistry } = this;
+ const { minRatioX, minRatioY } = this.getModuleProperties();
 
- if (isZoomLess(zoom, props.minRatioX, props.minRatioY)) {
+ if (isZoomLess(zoom, minRatioX, minRatioY)) {
  contextMenuRegistry.disableAction(CONTEXT_ZOOM_ACTION_ID);
  } else {
  contextMenuRegistry.enableAction(CONTEXT_ZOOM_ACTION_ID);
@@ -64,14 +65,14 @@ export class ZoomContextMenu {
  }
  }
 
- private onZoomToHere({ event }: AgChartContextMenuEvent, props: ZoomProperties) {
- const { rect } = this;
- const { enabled, isScalingX, isScalingY, minRatioX, minRatioY } = props;
+ private onZoomToHere({ event }: AgChartContextMenuEvent) {
+ const rect = this.getRect();
+ const { enabled, isScalingX, isScalingY, minRatioX, minRatioY } = this.getModuleProperties();
 
  if (!enabled || !rect || !event || !event.target || !(event instanceof MouseEvent)) return;
 
  const zoom = definedZoomState(this.zoomManager.getZoom());
- const origin = pointToRatio(rect, event.clientX, event.clientY);
+ const origin = pointToRatio(rect, event.offsetX, event.offsetX);
 
  const scaledOriginX = origin.x * dx(zoom);
  const scaledOriginY = origin.y * dy(zoom);
@@ -90,14 +91,14 @@ export class ZoomContextMenu {
  this.updateZoom(constrainZoom(newZoom));
  }
 
- private onPanToHere({ event }: AgChartContextMenuEvent, props: ZoomProperties) {
- const { rect } = this;
- const { enabled } = props;
+ private onPanToHere({ event }: AgChartContextMenuEvent) {
+ const rect = this.getRect();
+ const { enabled } = this.getModuleProperties();
 
  if (!enabled || !rect || !event || !event.target || !(event instanceof MouseEvent)) return;
 
  const zoom = definedZoomState(this.zoomManager.getZoom());
- const origin = pointToRatio(rect, event.clientX, event.clientY);
+ const origin = pointToRatio(rect, event.offsetX, event.offsetY);
 
  const scaleX = dx(zoom);
  const scaleY = dy(zoom);

packages/ag-charts-website/e2e/context-menu.spec.ts

@@ -5,7 +5,7 @@ import { gotoExample, setupIntrinsicAssertions, toExamplePageUrls } from './util
 test.describe('context-menu', () => {
  setupIntrinsicAssertions();
 
- const testUrls = toExamplePageUrls('financial-charts', 'financial-charts-showcase');
+ const testUrls = toExamplePageUrls('zoom', 'zoom-min-visible-items');
 
  for (const { framework, url } of testUrls) {
  test.describe(`for ${framework}`, () => {
@@ -18,7 +18,7 @@ test.describe('context-menu', () => {
 
  await page.click('canvas', {
  button: 'right',
- position: { x: width / 2, y: height / 2 },
+ position: { x: width * (2 / 3), y: height / 2 },
  });
  await page.locator('.ag-chart-context-menu__item').filter({ hasText: 'Zoom to here' }).click();
  await expect(page).toHaveScreenshot('zoom-to-here.png', { animations: 'disabled' });