triggerMouseEvent now returns the triggered event#19
Merged
Conversation
This allows calling code to do additional tests on the event; see chartjs/Chart.js#10046
etimberg
approved these changes
Feb 12, 2022
kurkle
approved these changes
Feb 12, 2022
joshkel
added a commit
to joshkel/Chart.js
that referenced
this pull request
Mar 23, 2022
To get supporting work from chartjs/chartjs-test-utils#19
etimberg
pushed a commit
to chartjs/Chart.js
that referenced
this pull request
Mar 24, 2022
* Refactor get...Items functions to take events rather than positions To work toward exposing something like the get...Items functions. * Switch getAxisItems to use optimizedEvaluateItems optimizedEvaluateItems falls back to evaluating all items for unsorted items, and sorting / optimizing ought to be okay, so this ought to be equivalent. * Performance * Consolidate getRelativePosition helpers.dom.js's getRelativePosition already had logic to handle ChartEvent vs. Event (as demonstrated by the `native` check within `getCanvasPosition`), so it's redundant for core.interaction.js to have its own `native` check. Update `getRelativePosition` to use the same `native` check as core.interaction.js's version. As best as I can tell, the ChartEvent's x and y are populated from `getRelativePosition`, so the previous `getCanvasPosition` was effectively just duplicating `getRelativePosition'`s work. I added a test to verify this; it depends on a local, not-yet-submitted change in chartjs-test-utils' `triggerMouseEvent` to return the mouse event that it triggers. * Add an API to refactor duplicate isPointInArea * Rename and update JSDoc to prepare for making this public * Give functions a consistent, generic interface * Export functions for discussion * Code review feedback 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. * Code review feedback Return the event directly, to speed up the code a bit. Add JSDoc to help communicate its intent. Update various comments. * Finalize exports; add docs and TypeScript * Update src/helpers/helpers.dom.js * Update src/helpers/helpers.dom.js Only thing needed actually is the update of chartjs-test-utils to 0.4.0 * Bump chartjs-test-utils dependency To get supporting work from chartjs/chartjs-test-utils#19 Co-authored-by: Jukka Kurkela <jukka.kurkela@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This allows calling code to do additional tests on the event; see chartjs/Chart.js#10046