Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(xy): multilayer time axis #1430

Merged
merged 31 commits into from
Oct 26, 2021
Merged

feat(xy): multilayer time axis #1430

merged 31 commits into from
Oct 26, 2021

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Oct 18, 2021 •
edited
Loading

Summary

Implementation of the multilayer time axis, based on the timeslip prototype

image

Details

Some of the things still to do:

  • API: add config option to switch on/off the multilayer axis (thanks Marco for your commit for this)
  • UX bug: under some circumstances, missing ticks on the right edge of the time domain
  • UX issue: avoid rendering the rightmost tick label if it would breach the right edge of the Cartesian projection area (we may also have an option for rendering it if there's margin space)
  • UX issue: add a couple of time rasters for the decades (it's unlikely but many-year timelines need it), with/without labeled years
  • UX issue: solve the returing axis tick duplication on a couple of mocks
  • UX issue: append st/nd/rd/th to day-of-month numbers
  • UX issue: properly varying the tick lengths to match the timeslip prototype and avoid overlap of labels with ticks
  • UX issue: clearer hierarchy of grid line saliency similar to timeslip
  • UX issue: top placed multilayer axis
  • UX issue: less contrasty grid lines and smaller contrast (luma) differences between different time granularities
  • UX issue: sparser ticks/gridlines 1: ticks/gridlines should be no denser than bin width (minimum interval) of data
  • UX issue: sparser ticks/gridlines 2: chart width dependent minimum allowed tick pixel distance
  • UX issue: theme integration and ensuring layer and theme dependent tick lengths (or lack thereof), grid saliency etc. into the options as a quick and dirty data propagation, it should be reworked
  • testing: as a test, run all image tests that have a time axis with this option on, so we can analyze the image diffs for odd cases
  • testing: fix up unit tests, some of which got commented out in the previous PR
  • testing: ensuring that time zones get handled correctly
  • testing: addition of VRT cases for the various time resolutions (quite a few zoom levels from ms to decades, not all covered yet)
  • implementation detail: cleaning up visible_ticks.ts (remove temp ts-ignores etc.)
  • implementation detail: in a follow-up PR, unify the new timeslip chrono with your former timeslip chrono

Issues

Closes #1310
Fixes #929
Fixes #908

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • The :theme label has been added and the @elastic/eui-design team has been pinged when there are Theme API changes
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated
  • The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
  • Visual changes have been tested with all available themes including dark, light, eui-dark & eui-light

@monfera monfera added enhancement New feature or request wip work in progress :axis Axis related issue :xy Bar/Line/Area chart related labels Oct 18, 2021
@monfera monfera marked this pull request as draft October 18, 2021 09:39
@monfera monfera force-pushed the multilayer-time-axis branch from d448a8b to 328d209 Compare October 21, 2021 00:39
@monfera monfera marked this pull request as ready for review October 21, 2021 10:04
@monfera monfera removed the wip work in progress label Oct 21, 2021
@monfera monfera force-pushed the multilayer-time-axis branch 2 times, most recently from ebadfc9 to 609310d Compare October 22, 2021 09:29
@monfera monfera force-pushed the multilayer-time-axis branch from 609310d to 917ac6c Compare October 22, 2021 09:50
@monfera monfera force-pushed the multilayer-time-axis branch from 3b7c5a2 to 5d6e852 Compare October 25, 2021 10:37
@monfera
Copy link
Contributor Author

monfera commented Oct 25, 2021

Force pushed as the previous commit to the feature branch didn't show up in the PR and didn't start a CI run, due to some github bug

@monfera
Copy link
Contributor Author

monfera commented Oct 25, 2021

And that commit still doesn't show up in the PR...

@monfera monfera force-pushed the multilayer-time-axis branch from 5d6e852 to 5a4ee6c Compare October 25, 2021 11:26
@monfera monfera force-pushed the multilayer-time-axis branch from 9983e52 to 761d52f Compare October 25, 2021 14:43
markov00
markov00 approved these changes Oct 26, 2021
View reviewed changes
Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, there is no major changes on the single layer time axis so it's good to merge

@monfera monfera merged commit 3d25854 into elastic:master Oct 26, 2021
nickofthyme pushed a commit that referenced this pull request Oct 26, 2021
@semantic-release-bot
chore(release): 38.1.0 [skip ci]
de28b09 
# [38.1.0](v38.0.1...v38.1.0) (2021-10-26)

### Bug Fixes

* **partition:** add get cursor pointer over slices ([#1428](#1428)) ([af776ae](af776ae))

### Features

* **xy:** multilayer time axis ([#1430](#1430)) ([3d25854](3d25854))
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 38.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Oct 26, 2021
@monfera monfera mentioned this pull request Oct 27, 2021
Open
11 tasks
@nickofthyme nickofthyme linked an issue Dec 9, 2021 that may be closed by this pull request
Improve axis ticks deduplication #1417
Closed
2 tasks
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 38.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markov00 markov00 markov00 approved these changes

Assignees
No one assigned
Labels
:axis Axis related issue enhancement New feature or request released on @38.1.x released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
Milestone
No milestone
3 participants
@monfera @nickofthyme @markov00