[8.19] [Lens] "Compare to" badge for Metric chart (#214811)#221709
Merged
nickofthyme merged 7 commits intoelastic:8.19from Jun 3, 2025
Merged
[8.19] [Lens] "Compare to" badge for Metric chart (#214811)#221709nickofthyme merged 7 commits intoelastic:8.19from
nickofthyme merged 7 commits intoelastic:8.19from
Conversation
27 tasks
Fixes elastic#184907 This PR introduces the Trend badge feature into the Secondary Metric chart type. <img width="1318" alt="Screenshot 2025-04-30 at 13 04 36" src="https://github.com/user-attachments/assets/6b3c599e-304f-4bc0-8ff2-094ed54ea248" /> <img width="1503" alt="Screenshot 2025-04-30 at 13 04 03" src="https://github.com/user-attachments/assets/cb203156-a7a8-4f61-bf30-57f7d1484fed" /> <img width="1339" alt="Screenshot 2025-04-30 at 13 03 21" src="https://github.com/user-attachments/assets/8896af7f-9193-41b5-a4b5-970907b084c5" /> <img width="1319" alt="Screenshot 2025-04-30 at 13 05 34" src="https://github.com/user-attachments/assets/12aa556e-8743-4167-9902-14fa9ef2e85d" /> Here's a short list of tasks handled by this PR: * [x] New Color by value option: `None` | `Static` | `Dynamic` * Default `None` option is the default (legacy preserved) * [x] `Static` option enables the badge with the picked color * [x] this is also enforced when the Primary metric is not number based (or when primary transition from number to text) * [x] `Dynamic` option enables both the badge coloring and the icon * [x] When disabled a tooltip with the explanation of the issue appears * [x] Trend palette is enabled by default on enable * [x] Added other 3 extra palettes (`Reversed`, `Temperature`, `Complementary`) * [x] Each palette has been implemented using both EUI tokens AND Vis tokens * [x] Revisited this decision ♻️ * [x] Remove redundant palettes * ~~keep only Vis palettes & hardcode `green` value for now.~~ * ~~Once EUI exposes the green token integrate in a follow up.~~ * Use the `@kbn/palettes` service with the new `compare_to` palette with hardcoded values * [x] Exposes display options: `Icon` | `Value` | `Both` * [x] `Both` is the default * [x] `Icon` or `Both` will show up 3 possible icon options: arrow up, arrow down and equal sign from unicode * [x] Make sure both text and icon respect breakpoint sizes * [x] Remove hack for breakpoints ( Lens Embeddable + Secondary metric) once new Charts upgrade is merged: elastic/elastic-charts#2627 * [x] Remove `theme` and contrast color computation once the Elastic Charts PR above is merged * [x] Make sure badge is always accessible * [x] Exposes Trend options: `Static value` | `Primary metric` * [x] `Static value` will show up a new `Baseline` control (debounced) * [x] `Primary metric` is enabled only for numeric primary values * [x] When disabled a tooltip shows up explaining the issue * [x] Added description text to explain difference with the other compare mode * [x] Show only delta value when in Primary * [x] Migrated existing `metric_vis` tests from Enzyme to RTL * [x] New tests for the new Secondary metric component in RTL * [x] Added new smoketest + panel mode caching testing for the metric feature in FTR Few aspects of this feature have been discussed and deferred to a follow up issue: elastic#217992 - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>
nickofthyme
force-pushed
the
backport/8.19/pr-214811
branch
from
0a571ba to
94af340
Compare
nickofthyme
commented
Jun 3, 2025
| ); | ||
| } | ||
| return formattedValue; | ||
| return formattedValue ?? null; |
Contributor
Author
There was a problem hiding this comment.
For some reason this does not matter in main but here returning undefined triggers this error...
SecondaryMetricValue(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.
| // Test id is the last resource here as the element will be empty | ||
| const el = screen.getByTestId('metric-secondary-element'); | ||
| expect(el).toBeEmptyDOMElement(); | ||
| expect(el).toContainHTML(''); |
Contributor
Author
There was a problem hiding this comment.
This element is returning this...
<span
data-test-subj="metric-secondary-element"
>
</span>Which according to the toBeEmptyDOMElement docs...
It ignores comments but will fail if the element contains white-space.
Replaced with toContainHTML.
Contributor
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
History
|
markov00
approved these changes
Jun 3, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Backport
This will backport the following commits from
mainto8.19:Questions ?
Please refer to the Backport tool documentation