[APM] Use same colors for HTTP status everywhere #47915
Conversation
💚 Build Succeeded
|
smith
added
v7.5.0
release_note:skip
|
|
||
| const httpStatusCodeColors: StringMap<string> = { | ||
| 1: euiColorDarkShade, | ||
| 2: euiColorSecondary, |
There was a problem hiding this comment.
For "2xx" what do you think about using this instead of euiColorSecondary ?
import { darken } from "polished";
import { palettes } from "@elastic/eui/lib/services";
const color2xx = darken(0.2, palettes.euiPaletteForStatus.colors[0]);Demo: https://codesandbox.io/s/http-status-colors-oxcpi
I think it works better as a "success" indicator, both in light and dark theme.
There was a problem hiding this comment.
If we're keen to have custom colors for HTTP status, wouldn't it be better to specifically declare them in their own variable instead of passing in the EUI colors, which might change over time. We can grab the hex colors and just create our own set, and set the 2xx to the custom color you grabbed ^
There was a problem hiding this comment.
I'm good with that too. It just felt a bit more okay to extend the palette like this.
There was a problem hiding this comment.
Why are we creating our own success color? Primary is fine I guess but if you want something more consistent with the rest of the UI why aren't we using euiColorSuccess ( #017d73)?
There was a problem hiding this comment.
euiColorSuccess and euiColorSecondary are more blue-teal than green. Especially in dark mode they look very alien compared to the yellow warning and red error colors we have ("successColor" is the custom color I'm suggesting)
Light:
Dark:
There was a problem hiding this comment.
If we are worried about the custom color, using the default will also work:
const color2xx = palettes.euiPaletteForStatus.colors[0];
|
@elasticmachine merge upstream |
💚 Build Succeeded
|
Use the same colors on the RPM/TPM graphs as in the status code badge. Fixes elastic#47817.
smith
force-pushed
the
nls/47817/http-colors
branch
from
9613d58 to
ed36dc3
Compare
💔 Build Failed |
💚 Build Succeeded |
* [APM] Use same colors for HTTP status everywhere Use the same colors on the RPM/TPM graphs as in the status code badge. Fixes elastic#47817. * don't export that const * Change colors * Remove unused import
|
Test plan: verified ✅
|
sorenlouv
added
the
apm:test-plan-done
Use the same colors on the RPM/TPM graphs as in the status code badge.
Fixes #47817.