-
Notifications
You must be signed in to change notification settings - Fork 122
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
fix(metric): background colors and sparkline rendering #2255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nickofthyme
added
bug
Something isn't working
:styling
Styling related issue
:metric
Related to Metric chart
labels
Nov 20, 2023
buildkite update screenshots |
nickofthyme
commented
Nov 20, 2023
@@ -40,5 +40,5 @@ export function fillTextColor( | |||
return highContrastColor(blendedFgBg, 'WCAG2', options); | |||
} | |||
|
|||
return highContrastColor(backgroundRGBA); | |||
return highContrastColor(backgroundRGBA, 'WCAG2', options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this last time when passing text contrast options
buildkite test this |
3 tasks
nickofthyme
added a commit
to nickofthyme/elastic-charts
that referenced
this pull request
Nov 20, 2023
# Conflicts: # e2e/screenshots/all.test.ts-snapshots/baselines/metric-alpha/grid-chrome-linux.png # e2e/screenshots/chart.test.ts-snapshots/chart/sizing/should-accommodate-chart-title-and-description-metric-sm-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-none-type/dark-theme/should-render-metric-with-click-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-none-type/dark-theme/should-render-metric-with-hover-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/dark-theme/should-render-metric-with-click-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/dark-theme/should-render-metric-with-hover-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/dark-theme/should-render-metric-with-transparent-bg-color-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/light-theme/should-render-metric-with-click-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/light-theme/should-render-metric-with-hover-interaction-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/metric-trend-type/light-theme/should-render-metric-with-transparent-bg-color-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-horizontal-progress-bar-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-horizontal-progress-bar-in-dark-mode-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-no-progress-bar-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-no-progress-bar-in-dark-mode-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-vertical-progress-bar-in-dark-mode-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/should-render-with-empty-and-missing-background-colors-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/text-value-with-trend-chrome-linux.png # e2e/screenshots/metric_stories.test.ts-snapshots/metric/value-icon-and-value-color-chrome-linux.png # packages/charts/src/chart_types/metric/renderer/dom/metric.tsx
nickofthyme
added a commit
to nickofthyme/elastic-charts
that referenced
this pull request
Nov 29, 2023
nickofthyme
pushed a commit
that referenced
this pull request
Dec 19, 2023
# [61.2.0](v61.1.0...v61.2.0) (2023-12-19) ### Bug Fixes * **deps:** update dependency @elastic/eui to ^91.1.0 ([#2267](#2267)) ([308e974](308e974)) * **deps:** update dependency @elastic/eui to ^91.2.0 ([#2268](#2268)) ([29cdcb3](29cdcb3)) * **metric:** background colors and sparkline rendering ([#2255](#2255)) ([5abddfc](5abddfc)) * **partition:** rendering with small radius ([#2273](#2273)) ([95a8537](95a8537)) * **partition:** zero value sectors cause max stack call ([#2260](#2260)) ([4b30db7](4b30db7)) * **theme:** legacy margins ([#2262](#2262)) ([299c869](299c869)) ### Features * increase tooltip width to 500px and truncate items to 2 lines ([#2261](#2261)) ([afdef1c](afdef1c))
1 task
markov00
added a commit
to elastic/kibana
that referenced
this pull request
Dec 21, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@elastic/charts](https://github.com/elastic/elastic-charts) | [`61.0.3` -> `61.2.0`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/61.0.3/61.2.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>elastic/elastic-charts (@​elastic/charts)</summary> ### [61.2.0](elastic/elastic-charts@v61.1.0...v61.2.0) (2023-12-19) ##### Bug Fixes * **deps:** update dependency @elastic/eui to ^91.1.0 ([#2267](elastic/elastic-charts#2267)) ([308e974](elastic/elastic-charts@308e974)) * **deps:** update dependency @elastic/eui to ^91.2.0 ([#2268](elastic/elastic-charts#2268)) ([29cdcb3](elastic/elastic-charts@29cdcb3)) * **metric:** background colors and sparkline rendering ([#2255](elastic/elastic-charts#2255)) ([5abddfc](elastic/elastic-charts@5abddfc)) * **partition:** rendering with small radius ([#2273](elastic/elastic-charts#2273)) ([95a8537](elastic/elastic-charts@95a8537)) * **partition:** zero value sectors cause max stack call ([#2260](elastic/elastic-charts#2260)) ([4b30db7](elastic/elastic-charts@4b30db7)) * **theme:** legacy margins ([#2262](elastic/elastic-charts#2262)) ([299c869](elastic/elastic-charts@299c869)) ##### Features * increase tooltip width to 500px and truncate items to 2 lines ([#2261](elastic/elastic-charts#2261)) ([afdef1c](elastic/elastic-charts@afdef1c)) ### [`v61.1.0`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6110-2023-11-20) [Compare Source](https://github.com/elastic/elastic-charts/compare/v61.0.3...v61.1.0) ##### Bug Fixes - **deps:** update dependency [@​elastic/eui](https://github.com/elastic/eui) to v91 ([#​2233](https://github.com/elastic/elastic-charts/issues/2233)) ([e89f623](https://github.com/elastic/elastic-charts/commit/e89f623792312c4f6b609ebb975de0800f3c297e)) - **metric:** add option to set empty cell background color ([#​2247](https://github.com/elastic/elastic-charts/issues/2247)) ([6a9fb86](https://github.com/elastic/elastic-charts/commit/6a9fb86bee5212a47060c5070f260961097014b4)) - **metric:** background color for bar with interactions ([#​2248](https://github.com/elastic/elastic-charts/issues/2248)) ([dcb56fa](https://github.com/elastic/elastic-charts/commit/dcb56fa08540631a9b3b0e588352ee6daf3d54a0)) ##### Features - **bullet:** improve header layout and positioning ([#​2243](https://github.com/elastic/elastic-charts/issues/2243)) ([b3a95d2](https://github.com/elastic/elastic-charts/commit/b3a95d24fb02690ca6599622352c743c04624690)) - **bullet:** new design style and implementation ([#​2156](https://github.com/elastic/elastic-charts/issues/2156)) ([fca6cdd](https://github.com/elastic/elastic-charts/commit/fca6cdd5bc34a65c0792dbab7d756404bf43501b)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/elastic/kibana). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTAzLjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes issues created in #2184 related to rendering of the sparkline with transparent backgrounds. Additions include:
Theme.metric.blendingBackground
option used to blend transparent colors when used with a non-default background.background
optionMetricBase
to override the background of individual metric cells.Details
In #2184 we added support for transparent colors. For this change we rendered the sparkline using an
svg
mask
to separate the fill from the sparkline (see #2184 (comment)) to allow both to have a transparent background that would then blend with theechBackground
color. But this approach created a small rendering artifact (see #2218) and also created a rendering issue with duplicated masking ids...The changes in #2184 created other issues (see #2248) causing a revert back to filling the metric elements with blend opaque colors. And thus the need to separate the sparkline svg elements is no longer valid. We now fill the svg sparkline with the respective blended background color.
Additionally we added the
Theme.metric.blendingBackground
option to specify the background color to blend transparent colors. This is useful in cases where the metric has a set background color like gray but has a transparent color fill. This option allows setting the blend color the the empty color of the theme rather than the gray background for more theme-specific coloring.Also fixes error in lightness logic with alpha values.
Checklist
:xy
,:partition
):interactions
,:axis
)packages/charts/src/index.ts
light
anddark
themes