Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Replaces jQuery's $el.width() with element.clientWidth. #180523

Merged
merged 13 commits into from
Apr 23, 2024

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Apr 10, 2024

Summary

Gets rid of leftover usage of jQuery. Replaces jQuery $el.width() with element.clientWidth that was used in the Anomaly Explorer chart for rare detectors.

I updated the jest test for the component to return timestamps that result in a reasonable amount of ticks. A jest spy was added to mock clientWidth which is not available via jsdom because it lacks a layout engine.

Checklist

@walterra walterra added :ml Feature:Anomaly Detection ML anomaly detection technical debt Improvement of the software architecture and operational architecture v8.14.0 labels Apr 10, 2024
@walterra walterra self-assigned this Apr 10, 2024
@walterra walterra requested a review from a team as a code owner April 10, 2024 18:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra added the release_note:skip Skip the PR/issue when compiling release notes label Apr 10, 2024
@walterra walterra requested review from peteharverson and removed request for darnautov April 11, 2024 12:46
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -46,6 +46,7 @@ describe('ExplorerChart', () => {
const originalGetBBox = SVGElement.prototype.getBBox;
beforeEach(() => (SVGElement.prototype.getBBox = () => mockedGetBBox));
afterEach(() => (SVGElement.prototype.getBBox = originalGetBBox));
jest.spyOn(Element.prototype, 'clientWidth', 'get').mockImplementation(() => 500);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
jest.spyOn(Element.prototype, 'clientWidth', 'get').mockImplementation(() => 500);
jest.spyOn(Element.prototype, 'clientWidth', 'get').mockReturnValue(500);

@walterra walterra requested review from a team as code owners April 22, 2024 12:29
@walterra
Copy link
Contributor Author

Note for for operations/QA team about the review ping: I had to change some files to investigate a problem where jest assertions are not the same on my local machine vs. CI. I will revert the changes once the problem is solved.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 4.1MB 4.1MB -83.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 77.3KB 77.2KB -51.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @walterra

@walterra walterra added v8.15.0 and removed v8.14.0 labels Apr 23, 2024
@walterra walterra merged commit 9ce171c into elastic:main Apr 23, 2024
18 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 23, 2024
@walterra walterra deleted the ml-remove-jquery branch April 23, 2024 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Anomaly Detection ML anomaly detection :ml release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants