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] Fix obscured service map connections #67129

Merged

Conversation

ogupte
Copy link
Contributor

@ogupte ogupte commented May 20, 2020

Closes #67126 by increasing spacingFactor in the Cytoscape layout from 0.85 -> 1.2

After (spacingFactor: 1.2):
Screen Shot 2020-05-20 at 4 59 12 PM

Before (spacingFactor: 0.85):
Screen Shot 2020-05-20 at 1 46 09 PM

@ogupte ogupte added release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.9.0 apm:test-plan-7.8.0 labels May 20, 2020
@ogupte ogupte requested a review from a team as a code owner May 20, 2020 16:50
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label May 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@@ -80,7 +80,7 @@ function getLayoutOptions(
roots: selectedRoots.length ? selectedRoots : undefined,
fit: true,
padding: nodeHeight,
spacingFactor: 0.85,
spacingFactor: 1.2,
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a magic number but if it works...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docs for this setting (https://js.cytoscape.org/#layouts/breadthfirst) describe it as:

positive spacing factor, larger => more space between nodes

Copy link
Member

Choose a reason for hiding this comment

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

All right 👍

@ogupte ogupte requested a review from nehaduggal May 20, 2020 21:19
@ogupte
Copy link
Contributor Author

ogupte commented May 21, 2020

retest

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@ogupte ogupte merged commit 9e39a13 into elastic:master May 21, 2020
ogupte added a commit to ogupte/kibana that referenced this pull request May 21, 2020
* Closes elastic#67126 by increasing spacingFactor in the cytoscape layout from 0.85 -> 1.2

* - prevents labels from hiding when nodes get a bit smaller
- also bump the label font size
ogupte added a commit to ogupte/kibana that referenced this pull request May 21, 2020
* Closes elastic#67126 by increasing spacingFactor in the cytoscape layout from 0.85 -> 1.2

* - prevents labels from hiding when nodes get a bit smaller
- also bump the label font size
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 21, 2020
* master: (21 commits)
  [Alerting] Hides the `alert` SavedObjects type (elastic#66719)
  skip flaky suite (elastic#66869)
  fix visual baseline tests
  [kbn/optimizer] require fsevents on macos (elastic#67147)
  [APM] Fix obscured service map connections (elastic#67129)
  [apm] Annotation API documentation (elastic#65963)
  [Uptime] Improve responsiveness details page (elastic#67034)
  skip flaky suite (elastic#66669)
  Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124)
  Support api_integration/kibana/stats against remote hosts (elastic#53000)
  chore(NA): add module name mapper for src plugins on x-pack (elastic#67103)
  Change the error message on TSVB in order to be more user friendly (elastic#67090)
  [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059)
  [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732)
  Bump styled-component dependencies (elastic#66611)
  Bump react-markdown dependencies (elastic#66615)
  Fix Core docs links (elastic#66977)
  Timelion graph is not refreshing content after searching or filtering (elastic#67023)
  Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053)
  Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432)
  ...
ogupte added a commit that referenced this pull request May 21, 2020
* Closes #67126 by increasing spacingFactor in the cytoscape layout from 0.85 -> 1.2

* - prevents labels from hiding when nodes get a bit smaller
- also bump the label font size
ogupte added a commit that referenced this pull request May 21, 2020
* Closes #67126 by increasing spacingFactor in the cytoscape layout from 0.85 -> 1.2

* - prevents labels from hiding when nodes get a bit smaller
- also bump the label font size
@dgieselaar dgieselaar self-assigned this May 26, 2020
@dgieselaar
Copy link
Member

Looks good, on Chrome, FF and Safari.

@dgieselaar dgieselaar added the apm:test-plan-done Pull request that was successfully tested during the test plan label May 26, 2020
@dgieselaar dgieselaar removed their assignment May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-7.8.0 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 Team:APM All issues that need APM UI Team support v7.8.0 v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Service map layout hides connections when vertical space is limited
6 participants