Skip to content

[7.16] fix crosshair theme and style issues#117180

Merged
nickofthyme merged 5 commits intoelastic:7.16from
nickofthyme:fix-crosshair-theme-issues
Nov 3, 2021
Merged

[7.16] fix crosshair theme and style issues#117180
nickofthyme merged 5 commits intoelastic:7.16from
nickofthyme:fix-crosshair-theme-issues

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Nov 2, 2021

Summary

Fixes issues related to cursor sync, position and theming. See elastic/elastic-charts#1452, elastic/elastic-charts#1447 and elastic/elastic-charts#1453.

Screen Recording 2021-11-02 at 11 40 28 AM

fixes #116754

Fixed in 8.0 in #117323

@nickofthyme nickofthyme added Feature:TSVB TSVB (Time Series Visual Builder) release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v7.16.0 labels Nov 2, 2021
@nickofthyme nickofthyme linked an issue Nov 2, 2021 that may be closed by this pull request
@nickofthyme
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Changes LGTM. I tested it locally by creating a dashboard of:

  • TSVB panel
  • Lens panel
  • XY vertical bar panel
  • Area vertical bar panel
  • Timelion

Now the crosshair is visible to all of them 🎉
The only thing I don't get is why we are backporting it only in 7.16? I mean this should be fixed for 8 and above, right?
I saw a PR from Marco that is updating the version on 8.0 and 8.1 but doesn't include all your changes.

@markov00
Copy link
Contributor

markov00 commented Nov 3, 2021

@stratoula

The only thing I don't get is why we are backporting it only in 7.16? I mean this should be fixed for 8 and above, right?
I saw a PR from Marco that is updating the version on 8.0 and 8.1 but doesn't include all your changes.

I will create a new PR for 8.0 when #116749 is merged with a new ech release that includes those changes. Unfortunately we can't just merge the latest version of ECH in 7.16 due to the features introduced in 38.1 in charts. We backported the changes in 38.0 and we will create a new PR for 8.0/8.1 with 38.1 fixes

@stratoula
Copy link
Contributor

Oh nice, this makes perfect sense! Thanx!

@nickofthyme
Copy link
Contributor Author

@stratoula yeah both have the three changes listed above, the other one likely just got released in an earlier version.

I think @markov00 first mentioned adding it to 7.16 in elastic/elastic-charts#1452 (review), but I think the changes are not very risky and there is almost a month before it's released. But if you see different I'm fine adding it just to 8.0.

@stratoula
Copy link
Contributor

We want it in 7.16!! It works correctly and it doesn't seem risky

@nickofthyme nickofthyme enabled auto-merge (squash) November 3, 2021 14:36
@nickofthyme nickofthyme requested a review from alexwizp November 3, 2021 14:37
Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you

@nickofthyme
Copy link
Contributor Author

@elasticmachine merge upstream

@nickofthyme nickofthyme merged commit 53604df into elastic:7.16 Nov 3, 2021
@kibanamachine
Copy link
Contributor

💚 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
visTypeTimeseries 638.4KB 638.4KB -27.0B
visTypeXy 61.9KB 61.8KB -27.0B
total -54.0B

Page load bundle

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

id before after diff
kbnUiSharedDeps-npmDll 5.0MB 5.0MB +3.6KB

History

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

@nickofthyme nickofthyme deleted the fix-crosshair-theme-issues branch November 5, 2021 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:TSVB TSVB (Time Series Visual Builder) release_note:skip Skip the PR/issue when compiling release notes v7.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TSVB][Timelion][XY] Cursor synchronization is not visible

5 participants