[Lens] Reference line renaming + other small fixes #113811

dej611 · 2021-10-04T17:34:40Z

Summary

This PR is a combination of all the previous PRs (also the pending ones) made in preparation for the renaming Thresholds => Reference Line.

The mapping is almost 1:1, with few exceptions where Reference lines X was too long, so I've changed it to Reference X. One instance of it is the Add layer button:

During the change, I've found out few minor bugs which have been addressed here (but they can be ported elsewhere as well if it improves the review process):

Formula to Static value transition was broken again, so it has been fixed ✅
Fill: below was not respecting the "next reference line" boundary, leading to mixed color region. ✅
Fill options did not change for Horizontal Charts, or Vertical lines. Now Above and Below change to Before and After ✅
The invalidMessage for a number histogram axis change or removed has been improved for the specific case ✅
Better vertical text alignment ✅ - will be ported also to [Lens] Thresholds: Add text to markers body #113629

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…reshold_panel.tsx Co-authored-by: Michael Marcialis <[email protected]>

elasticmachine · 2021-10-14T15:55:56Z

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

dej611 · 2021-10-14T17:18:19Z

@elasticmachine merge upstream

stratoula

I am not sure if it is supposed to work like that but I notice that when I create a reference line with a Quick function and then go to Formula this works fine. When I go to the static value, the value is overwritten by 100. I would expect to carry the value that is computed by the Quck function.

When I navigate to static value

stratoula · 2021-10-15T08:59:32Z

x-pack/plugins/lens/public/xy_visualization/xy_config_panel/layer_header.tsx

-        label={i18n.translate('xpack.lens.xyChart.layerThresholdLabel', {
-          defaultMessage: 'Thresholds',
+        icon={LensIconChartBarReferenceLine}
+        label={i18n.translate('xpack.lens.xyChart.layerReference lineLabel', {


Not sure about it but is this ok that it has a space on the key?

Nice catch! It's a typo.

stratoula · 2021-10-15T09:00:35Z

x-pack/plugins/lens/public/xy_visualization/xy_config_panel/reference_line_panel.tsx

+  const beforeLabel = i18n.translate('xpack.lens.xyChart.referenceLineFill.before', {
+    defaultMessage: 'Before',
+  });
+  const afterLabel = i18n.translate('xpack.lens.xyChart.referenceLineFill.right', {


nit: this could be xpack.lens.xyChart.referenceLineFill.after in order to be aligned with the rest

dej611 · 2021-10-15T12:32:31Z

The transition bug from Quick functions should have been fixed now.
Addressed also the other two issues.

mdefazio

Reviewed scss file and LGTM.

stratoula

Code LGTM, the bug with the transition from the Quick functions to static has been solved. We could also update the Display name here:

but I am approving it!

dej611 · 2021-10-15T13:38:18Z

Fixed also latest issue with label

kibanamachine · 2021-10-15T15:38:32Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`lens`	1.0MB	1.0MB	+1.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lens`	39.2KB	39.2KB	+8.0B

History

💚 Build #161379 succeeded 974db2e
💚 Build #161067 succeeded fb8604e
💔 Build #161005 failed c5de0d2
💚 Build #160337 succeeded deb6185
💚 Build #160122 succeeded c921d8b

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

* 🐛 Add padding to the tick label to fit threshold markers * 🐛 Better icon detection * 🐛 Fix edge cases with no title or labels * 📸 Update snapshots * ✨ Make threshold fit into view automatically * 🐛 do not compute axis threshold extends if no threshold is present * ✅ One more fix for 0-based extends and tests * ✨ Add icon placement flag * ✨ Sync padding computation with marker positioning * ✨ compute the default threshold based on data bounds * 🐛 fix duplicate suggestion issue + missing over time * 👌 Make disabled when no icon is selected * ✨ First text on marker implementation * 🐛 Fix some edge cases with auto positioning * Update x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx Co-authored-by: Michael Marcialis <[email protected]> * 🐛 Fix minor details * 💄 Small tweak * ✨ Reduce the padding if no icon is shown on the axis * 🐛 Fix color fallback for different type of layers * ✅ Fix broken unit tests * 🐛 Fix multi layer types issue * ✅ Fix test * ✅ Fix other test * 💄 Fix vertical text centering * ✨ Rename to reference lines + few fixes * 🚨 Fix linting issue * 🐛 Fix issue * 🐛 Fix computation bug for the initial static value * ✅ Add new suite of test for static value computation * 💄 Reorder panel inputs * 💄 Move styling to sass * 📝 Keeping up with the renaming * ✅ Fix functional tests after renaming * 🐛 Fix duplicate arg from conflict resolution * 👌 Integrate some follow up feedback * 📝 Fix typo * 👌 Integrate feedback * 🐛 Fix the quick functions transition bug * 🐛 Fix label issue when updating value Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Marcialis <[email protected]>

kibanamachine · 2021-10-15T16:08:18Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* 🐛 Add padding to the tick label to fit threshold markers * 🐛 Better icon detection * 🐛 Fix edge cases with no title or labels * 📸 Update snapshots * ✨ Make threshold fit into view automatically * 🐛 do not compute axis threshold extends if no threshold is present * ✅ One more fix for 0-based extends and tests * ✨ Add icon placement flag * ✨ Sync padding computation with marker positioning * ✨ compute the default threshold based on data bounds * 🐛 fix duplicate suggestion issue + missing over time * 👌 Make disabled when no icon is selected * ✨ First text on marker implementation * 🐛 Fix some edge cases with auto positioning * Update x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx Co-authored-by: Michael Marcialis <[email protected]> * 🐛 Fix minor details * 💄 Small tweak * ✨ Reduce the padding if no icon is shown on the axis * 🐛 Fix color fallback for different type of layers * ✅ Fix broken unit tests * 🐛 Fix multi layer types issue * ✅ Fix test * ✅ Fix other test * 💄 Fix vertical text centering * ✨ Rename to reference lines + few fixes * 🚨 Fix linting issue * 🐛 Fix issue * 🐛 Fix computation bug for the initial static value * ✅ Add new suite of test for static value computation * 💄 Reorder panel inputs * 💄 Move styling to sass * 📝 Keeping up with the renaming * ✅ Fix functional tests after renaming * 🐛 Fix duplicate arg from conflict resolution * 👌 Integrate some follow up feedback * 📝 Fix typo * 👌 Integrate feedback * 🐛 Fix the quick functions transition bug * 🐛 Fix label issue when updating value Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Marcialis <[email protected]> Co-authored-by: Marco Liberati <[email protected]> Co-authored-by: Michael Marcialis <[email protected]>

dej611 and others added 30 commits September 23, 2021 12:09

🐛 Add padding to the tick label to fit threshold markers

d7f6363

🐛 Better icon detection

6f59341

🐛 Fix edge cases with no title or labels

a5c5c09

📸 Update snapshots

e705e0f

Merge branch 'master' into fix/112793

a61c78b

✨ Make threshold fit into view automatically

82dba7b

🐛 do not compute axis threshold extends if no threshold is present

f3616a3

✅ One more fix for 0-based extends and tests

e78cd2b

Merge branch 'master' into fix/112793

33845bb

✨ Add icon placement flag

5a5270a

✨ Sync padding computation with marker positioning

6cfad94

Merge branch 'master' into fix/112793

6485fa7

✨ compute the default threshold based on data bounds

45040f0

🐛 fix duplicate suggestion issue + missing over time

9d8cb7e

Merge branch 'master' into fix/112795

4b33f6c

Merge branch 'master' into fix/112788

1b2d56c

Merge branch 'master' into fix/112795

b0d5b4e

👌 Make disabled when no icon is selected

af96b0a

Merge branch 'master' into fix/112793

5a9ee3b

✨ First text on marker implementation

8eedc11

🐛 Fix some edge cases with auto positioning

44a70a4

Merge branch 'fix/112793' of github.com:dej611/kibana into fix/112793

fbb372c

Merge branch 'fix/112793' into fix/112921

21c60b7

Update x-pack/plugins/lens/public/xy_visualization/xy_config_panel/th…

5b4374c

…reshold_panel.tsx Co-authored-by: Michael Marcialis <[email protected]>

🐛 Fix minor details

20cecf4

Merge branch 'fix/112793' into fix/112921

c5b4e73

💄 Small tweak

f8bb856

Merge remote-tracking branch 'upstream/master' into fix/112793

82a83aa

Merge branch 'fix/112793' into fix/112921

43a3a7f

Merge remote-tracking branch 'upstream/master' into fix/112793

ac46286

dej611 added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.0 v8.0.0 labels Oct 14, 2021

dej611 marked this pull request as ready for review October 14, 2021 15:55

dej611 requested review from a team as code owners October 14, 2021 15:55

Merge branch 'master' into fix/113406

fb8604e

stratoula reviewed Oct 15, 2021

View reviewed changes

dej611 added 4 commits October 15, 2021 11:37

📝 Fix typo

9479213

👌 Integrate feedback

5b818ff

🐛 Fix the quick functions transition bug

8f62aab

Merge branch 'fix/113406' of github.com:dej611/kibana into fix/113406

974db2e

mdefazio approved these changes Oct 15, 2021

View reviewed changes

stratoula approved these changes Oct 15, 2021

View reviewed changes

🐛 Fix label issue when updating value

d6d004b

dej611 merged commit 9d12a97 into elastic:master Oct 15, 2021

dej611 deleted the fix/113406 branch October 15, 2021 16:06

kibanamachine mentioned this pull request Oct 15, 2021

[7.x] [Lens] Reference line renaming + other small fixes (#113811) #115225

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Reference line renaming + other small fixes #113811

[Lens] Reference line renaming + other small fixes #113811

dej611 commented Oct 4, 2021

elasticmachine commented Oct 14, 2021

dej611 commented Oct 14, 2021

stratoula left a comment

stratoula Oct 15, 2021

dej611 Oct 15, 2021

stratoula Oct 15, 2021

dej611 commented Oct 15, 2021

mdefazio left a comment

stratoula left a comment

dej611 commented Oct 15, 2021

kibanamachine commented Oct 15, 2021

kibanamachine commented Oct 15, 2021

[Lens] Reference line renaming + other small fixes #113811

[Lens] Reference line renaming + other small fixes #113811

Conversation

dej611 commented Oct 4, 2021

Summary

Checklist

Risk Matrix

For maintainers

elasticmachine commented Oct 14, 2021

dej611 commented Oct 14, 2021

stratoula left a comment

Choose a reason for hiding this comment

stratoula Oct 15, 2021

Choose a reason for hiding this comment

dej611 Oct 15, 2021

Choose a reason for hiding this comment

stratoula Oct 15, 2021

Choose a reason for hiding this comment

dej611 commented Oct 15, 2021

mdefazio left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

dej611 commented Oct 15, 2021

kibanamachine commented Oct 15, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

kibanamachine commented Oct 15, 2021

💚 Backport successful