Skip to content

Commit

Permalink
Code review feedback
Browse files Browse the repository at this point in the history
Add a non-null assertion, as requested in code review.

Add JSDoc to clarify that `getCanvasPosition` now expects a native `Event`, not a `ChartEvent`.  Add `@ts-ignore`; `getCanvasPosition` relied on some loose conversions between `Event`, `TouchEvent`, and `Touch` that would require several changes to make TypeScript happy.
  • Loading branch information
joshkel committed Feb 14, 2022
1 parent 121167d commit 0f99ce4
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/helpers/helpers.dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ function getPositionedStyle(styles, style, suffix) {

const useOffsetPos = (x, y, target) => (x > 0 || y > 0) && (!target || !target.shadowRoot);

/**
* @param {Event} e
* @param {HTMLCanvasElement} canvas
* @returns {{x: number, y: number, box: boolean}}
*/
function getCanvasPosition(e, canvas) {
// @ts-ignore
const touches = e.touches;
const source = touches && touches.length ? touches[0] : e;
const {offsetX, offsetY} = source;
Expand Down
1 change: 1 addition & 0 deletions test/specs/helpers.dom.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ describe('DOM helpers tests', function() {
const nativeEvent = await jasmine.triggerMouseEvent(chart, 'mousemove', point);
const nativePosition = Chart.helpers.getRelativePosition(nativeEvent, chart);

expect(chartPosition).not.toBeNull();
expect(nativePosition).toEqual(chartPosition);
});
});
Expand Down

0 comments on commit 0f99ce4

Please sign in to comment.