[charts] Fix multilayer time axis styles by markov00 · Pull Request #116749 · elastic/kibana

markov00 · 2021-10-29T10:29:34Z

Summary

This PR fixes a set of issues to the new multilayer time axis:

the tickLine is now removed from ticks without a label
the axis/tick/label style is restored to the original EUI one
the multilayer time axis style is now moved into the charts plugin and reused
Lens: use the single-layer time axis when bars cluster is used
TSVB: I reduced a bit the number of ticks on the Y axis, to reduce the noise of gridlines with multilayer axis
Discover: I reduced by 8px the height of the histogram and moved the top padding to the bottom to separate a bit the time range text and the time axis

Coming from @elastic/charts update:

multilayer time axis tick/grid is shown only when tick is inside domain (this removes the black vertical axis line at the beginning of the chart) fix(xy): show mouse cursors on charts with opaque background elastic-charts#1447
Fix the invisible cursor on charts with opaque backgrounds fix(xy): multilayer time axis tick/grid only when tick is inside domain elastic-charts#1446
Add missing last tick and rarify gridlines fix(xy): adding missing last tick and other tick improvements elastic-charts#1448

markov00 · 2021-11-01T09:16:02Z

@elasticmachine merge upstream

elasticmachine · 2021-11-01T17:08:17Z

Pinging @elastic/datavis (Feature:ElasticCharts)

x-pack/plugins/uptime/public/components/common/charts/__snapshots__/donut_chart.test.tsx.snap

elasticmachine · 2021-11-01T17:42:59Z

Pinging @elastic/uptime (Team:uptime)

dominiqueclarke

Uptime changes LGTM

kertal

Data Discover team owned code LGTM; tested Discover a-la-carte, works as expected

stratoula · 2021-11-03T08:37:34Z

@elasticmachine merge upstream

stratoula

It looks fine, the only thing I noticed is that a TSVB area chart doesn't have the new axis.

Moreover, we could also enable the timeAxis for Timelion (irrelevant with this PR but just a thought). Wdyt?

stratoula · 2021-11-03T09:58:57Z

Another thing I noticed is that the labels orientation doesn't work for the new axis

Maybe it makes sense to disable them?

markov00 · 2021-11-03T10:41:04Z

It looks fine, the only thing I noticed is that a TSVB area chart doesn't have the new axis.

You are right, I've pushed a fix here 6bcad3f

Moreover, we could also enable the timeAxis for Timelion (irrelevant with this PR but just a thought). Wdyt?

Not right now and not for 8.0, the timelion default parameters for bars are not suitable to be used with the multilayer time axis. We can enable it only for area/line charts in case. We are anyway after FF and we can't merge new features in actually.

Another thing I noticed is that the labels orientation doesn't work for the new axis !
Maybe it makes sense to disable them?

We are going to disable them but I prefer to disable them in a different PR, this one should be merged first as it is way more important for now

stratoula

@markov00 I totally agree. About timelion I was thinking to enable it on a future minor if it makes sense and I am fine to disable them on another PR, it is mostly for UX reasons.

Changes LGTM! I tested it locally and works fine in Lens, TSVB and. Visualize

kibanamachine · 2021-11-03T12:35:38Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`charts`	64	66	+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`charts`	254	278	+24

Async chunks

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

id	before	after	diff
`discover`	303.5KB	303.2KB	-373.0B
`lens`	953.9KB	953.8KB	-137.0B
`visTypeTimeseries`	443.7KB	443.4KB	-259.0B
`visTypeXy`	59.7KB	59.6KB	-100.0B
total			-869.0B

Page load bundle

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

id	before	after	diff
`charts`	56.7KB	57.1KB	+395.0B
`kbnUiSharedDeps-npmDll`	5.0MB	5.0MB	-79.0B
total			+316.0B

Unknown metric groups

API count

id	before	after	diff
`charts`	286	310	+24

History

💚 Build #3892 succeeded 9c42539
💚 Build #3255 succeeded cf16f97
💔 Build #3229 failed b7d27c5
💚 Build #3144 succeeded 9fe5a49
💚 Build #3119 succeeded a24dc38
💔 Build #2786 failed 34d3b18

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

This commit fixes a set of issues to the new multilayer time axis: - the tickLine is now removed from ticks without a label - the axis/tick/label style is restored to the original EUI one - the multilayer time axis style is now moved into the charts plugin and reused - Lens: use the single-layer time axis when bars cluster is used - TSVB: I reduced a bit the number of ticks on the Y axis, to reduce the noise of gridlines with multilayer axis - Discover: I reduced by 8px the height of the histogram and moved the top padding to the bottom to separate a bit the time range text and the time axis Coming from @elastic/charts update: - multilayer time axis tick/grid is shown only when tick is inside domain (this removes the black vertical axis line at the beginning of the chart) fix(xy): show mouse cursors on charts with opaque background elastic-charts#1447 - Fix the invisible cursor on charts with opaque backgrounds fix(xy): multilayer time axis tick/grid only when tick is inside domain elastic-charts#1446 - Add missing last tick and rarify gridlines fix(xy): adding missing last tick and other tick improvements elastic-charts#1448

kibanamachine · 2021-11-03T12:38:58Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

This commit fixes a set of issues to the new multilayer time axis: - the tickLine is now removed from ticks without a label - the axis/tick/label style is restored to the original EUI one - the multilayer time axis style is now moved into the charts plugin and reused - Lens: use the single-layer time axis when bars cluster is used - TSVB: I reduced a bit the number of ticks on the Y axis, to reduce the noise of gridlines with multilayer axis - Discover: I reduced by 8px the height of the histogram and moved the top padding to the bottom to separate a bit the time range text and the time axis Coming from @elastic/charts update: - multilayer time axis tick/grid is shown only when tick is inside domain (this removes the black vertical axis line at the beginning of the chart) fix(xy): show mouse cursors on charts with opaque background elastic-charts#1447 - Fix the invisible cursor on charts with opaque backgrounds fix(xy): multilayer time axis tick/grid only when tick is inside domain elastic-charts#1446 - Add missing last tick and rarify gridlines fix(xy): adding missing last tick and other tick improvements elastic-charts#1448 Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com>

markov00 added 5 commits October 29, 2021 10:48

Fix visualize tick lines

e99c89c

Fix histogram tick lines

d38e7ba

Fix Lens tick lines

6a90a3d

Fix TSVB tick line

4dffd0a

update ech to v38.1.2

34d3b18

markov00 changed the title ~~2021 10 28 fix multitime axis styles~~ [charts] Fixe multilayer time axis styles Oct 29, 2021

markov00 changed the title ~~[charts] Fixe multilayer time axis styles~~ [charts] Fix multilayer time axis styles Oct 29, 2021

brianseeders changed the base branch from main to master October 29, 2021 15:16

Fix Lens snapshot test after code changes

a90a553

kibanamachine and others added 2 commits November 1, 2021 05:16

Merge branch 'main' into 2021_10_28-fix_multitime_axis_styles

a24dc38

Update ECH to 38.1.3

9fe5a49

markov00 mentioned this pull request Nov 1, 2021

Kibana multi-layer time axis elastic/elastic-charts#1442

Closed

18 tasks

markov00 added 5 commits November 1, 2021 13:12

Reduce discover chart height and move the top padding to the bottom

85b2882

Fix TSVB max Y ticks and new time axis only if stacked

5f2f84e

DRY style and share it from charts plugin

b7d27c5

Fix Lens usage of multilayer time axis

5eeb3af

Fix style const name in visualize

cf16f97

markov00 marked this pull request as ready for review November 1, 2021 17:08

markov00 requested review from a team as code owners November 1, 2021 17:08

dominiqueclarke reviewed Nov 1, 2021

View reviewed changes

x-pack/plugins/uptime/public/components/common/charts/__snapshots__/donut_chart.test.tsx.snap Show resolved Hide resolved

botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Nov 1, 2021

dominiqueclarke approved these changes Nov 1, 2021

View reviewed changes

ryankeairns approved these changes Nov 1, 2021

View reviewed changes

kertal approved these changes Nov 2, 2021

View reviewed changes

Merge branch 'main' into 2021_10_28-fix_multitime_axis_styles

9c42539

stratoula reviewed Nov 3, 2021

View reviewed changes

Fix TSVB multilayer axis with lines or stacked bars

6bcad3f

markov00 mentioned this pull request Nov 3, 2021

[7.16] fix crosshair theme and style issues #117180

Merged

stratoula approved these changes Nov 3, 2021

View reviewed changes

markov00 enabled auto-merge (squash) November 3, 2021 11:00

markov00 merged commit 5fc70f5 into elastic:main Nov 3, 2021

kibanamachine mentioned this pull request Nov 3, 2021

[8.0] [charts] Fix multilayer time axis styles (#116749) #117296

Merged

markov00 added a commit to nickofthyme/kibana that referenced this pull request Dec 20, 2021

Cherry-pick tests fixes from PR elastic#116749 and elastic#115853

20dd1c3

Conversation

markov00 commented Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

markov00 commented Nov 1, 2021

Uh oh!

elasticmachine commented Nov 1, 2021

Uh oh!

Uh oh!

elasticmachine commented Nov 1, 2021

Uh oh!

dominiqueclarke left a comment

Choose a reason for hiding this comment

Uh oh!

kertal left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula commented Nov 3, 2021

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula commented Nov 3, 2021

Uh oh!

markov00 commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Nov 3, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

kibanamachine commented Nov 3, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

markov00 commented Oct 29, 2021 •

edited

Loading

markov00 commented Nov 3, 2021 •

edited

Loading