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

[CSM] Fix core vital legend background #78273

Merged
merged 14 commits into from
Sep 29, 2020

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Sep 23, 2020

Summary

Fixes: #77283

Use dark theme var when dark mode is selected for background color.

Added text alongside core vital average, added a popover help icon

image

@shahzad31 shahzad31 requested a review from a team as a code owner September 23, 2020 11:25
@botelastic botelastic bot added Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Sep 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@elasticmachine
Copy link
Contributor

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

@shahzad31 shahzad31 self-assigned this Sep 23, 2020
@shahzad31 shahzad31 added v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 23, 2020
@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Tested in dark mode and UI LGTM.

Left a bunch of comments but they're mostly optional or wellness checks.

@@ -28,7 +29,7 @@ export function CoreVitals({ data, loading }: Props) {
<EuiFlexItem>
<CoreVitalItem
title={LCP_LABEL}
value={lcp ? lcp + ' s' : '0'}
value={formatToSec(lcp, 'ms')}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the output of this function going to be a string presented to the user, or is 'ms' an enumerable string?

If this will cause ms to show to the user make sure the output is translated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is already done in another PR

lcp: ((lcp?.values['50.0'] || 0) / 1000).toFixed(2),
tbt: tbt?.values['50.0'] || 0,
fcp: fcp?.values['50.0'] || 0,
fid: fid?.values['50.0'] ?? 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this value have > 2 decimal digits? Ignore if the answer is no, or formatting it as such is no longer important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's done in frontend

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id value diff baseline
apm 4.1MB +5.7KB 4.1MB

History

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

@shahzad31 shahzad31 merged commit fdee5e5 into elastic:master Sep 29, 2020
@shahzad31 shahzad31 deleted the core-vital-legend-background branch September 29, 2020 11:54
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 29, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
shahzad31 added a commit that referenced this pull request Sep 29, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 29, 2020
…-to-timeline

* 'master' of github.com:elastic/kibana: (22 commits)
  update apm index pattern (elastic#78732)
  78024: move transform out of dataset (elastic#78216)
  [QA][Code Coverage] Upload the coverage static site before ingestion (elastic#78695)
  [Discover] Make _source field not clickable (elastic#78698)
  [Fleet] Rename Ingest Manager => Fleet, Fleet => Agents in the UI (elastic#78685)
  [APM] Review feedback from distribution + transaction metrics (elastic#78752)
  [Ingest pipelines] Add ability to stop pipeline simulation  (elastic#78183)
  [CSM] Fix core vital legend background (elastic#78273)
  [Usage Collection] [schema] Support spreads + `canvas` definition (elastic#78481)
  fix lodash imports (elastic#78456)
  [Maps] Add layer type preview icons (elastic#78650)
  [APM] Use transaction metrics for distribution charts (elastic#78484)
  [Uptime] Ml anomaly alert edit (elastic#76909)
  [ML] Limit exposing shared static code through ml/public/index.ts. (elastic#77745)
  making expression debug info serializable (elastic#78727)
  fix lodahs imports in app-arch code (elastic#78582)
  Make Field a React.lazy export (elastic#78483)
  [Security Solution] Improves detections tests (elastic#77295)
  [TSVB] Different field format on different series is ignored (elastic#78138)
  RFC: Improve saved object migrations (elastic#66056)
  ...
@sorenlouv sorenlouv removed the Team:APM All issues that need APM UI Team support label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RUM Dashboard] Hovering on RUM core web vitals percentages showing white background in dark mode
5 participants