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

[APM] Service maps IE 11 bug fixes #63558

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

ogupte
Copy link
Contributor

@ogupte ogupte commented Apr 15, 2020

Closes #63113 by limiting service maps to only draw certain shapes in IE 11 without icons. Also fixes flex bugs in the popup details.

IE11 after fixes:
Screen Shot 2020-04-15 at 2 00 39 AM
Screen Shot 2020-04-15 at 2 39 34 AM

@ogupte ogupte added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.8.0 apm-test-plan-7.7.0 labels Apr 15, 2020
@ogupte ogupte requested a review from a team as a code owner April 15, 2020 10:11
// https://stackoverflow.com/a/21825207
//
// @ts-ignore `documentMode` is not recognized as a valid property of `document`.
const isIE11 = !!window.MSInputMethodContext && !!document.documentMode;
Copy link
Member

Choose a reason for hiding this comment

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

we are using this in public/components/app/ServiceMap/icons.ts as well, should we put it in common and import it from there in both places?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, now that I think about it, the flag in icons.ts is now unnecessary since the iconForNode function is never called now (from cytoscapeOptions.ts. We can address this in another PR since this is already merged and backported now.

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

nice @ogupte!

@formgeist
Copy link
Contributor

@ogupte Not all that important, but if it's possible, can the service nodes be circles instead of squares to get closer to the real style?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@ogupte ogupte merged commit d1d0a44 into elastic:master Apr 15, 2020
ogupte added a commit to ogupte/kibana that referenced this pull request Apr 15, 2020
ogupte added a commit to ogupte/kibana that referenced this pull request Apr 15, 2020
ogupte added a commit that referenced this pull request Apr 15, 2020
wayneseymour pushed a commit that referenced this pull request Apr 15, 2020
ogupte added a commit that referenced this pull request Apr 15, 2020
… IE 11 without icons (#63558) (#63590)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@ogupte
Copy link
Contributor Author

ogupte commented Apr 15, 2020

@ogupte Not all that important, but if it's possible, can the service nodes be circles instead of squares to get closer to the real style?

@formgeist: Unfortunately, in IE 11, drawing circles were producing errors. The bug seemed to show itself when it traversed specific segments of a draw path for certain shapes. Ellipses (including circles) and triangles (connection arrows) were among those shapes with the kind of drawing path that produced the error, but diamonds, and rectangles (including squares) seemed to render without issue.

@formgeist
Copy link
Contributor

@formgeist: Unfortunately, in IE 11, drawing circles were producing errors. The bug seemed to show itself when it traversed specific segments of a draw path for certain shapes. Ellipses (including circles) and triangles (connection arrows) were among those shapes with the kind of drawing path that produced the error, but diamonds, and rectangles (including squares) seemed to render without issue.

OK, thanks for looking into it though 👍

@cauemarcondes cauemarcondes added the apm:test-plan-done Pull request that was successfully tested during the test plan label Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-done Pull request that was successfully tested during the test plan release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Service maps empty on IE11
5 participants