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.

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

feat(heatmap): small multiples #1933

Merged
merged 79 commits into from
Mar 10, 2023
Merged

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Jan 12, 2023

Summary

Adds support for small multiples for heatmap chart types. View demo story here.

Screen.Recording.2023-03-07.at.03.10.43.PM.mp4

BREAKING CHANGES

Removed unused properties maxColumnWidth and maxRowHeight under HeatmapStyle. Also removed grid height and width constraints (i.e. cellWidth and cellHeight) in favor of filled rendering and pagination driven by data from the consumer side.

@@ -1424,25 +1424,13 @@ export interface HeatmapStyle {
     grid: {
-        cellWidth: {
-            min: Pixels;
-            max: Pixels | 'fill';
-        };
-        cellHeight: {
-            min: Pixels;
-            max: Pixels | 'fill';
-        };
         stroke: {
             color: string;
             width: number;
         };
     };
-    maxColumnWidth: Pixels;
     maxLegendHeight?: number;
-    maxRowHeight: Pixels;

Details

Rational behind breaking changes -- The heatmap grid constraints were initially used to constrain the height on all rows or widths on all columns. This creates a problem as the overall chart size is not responsive to these grid constraints and would truncate the dataset as needed to respect the grid constraints. So in some cases you could have a heatmap with 20 rows of data but only say 10 would be visible, and there was no mechanism internally to charts to communicate the page nor page size to the chart consumer, thus creating an inaccessible and otherwise unknown truncation of the dataset.

Consumers in kibana using these constraints are computing the necessary overall chart height to set the rows to a constant value. Doing so would size the rows adequately with a simple fill height without the need for manual constraints.

Additionally, all other charts are sized based on the top-level container size to fill the available space with no sizing based on internal values or child element sizes.

The maxColumnWidth and maxRowHeight values were not being used anywhere thus have no impact to any kibana usage.

TODOS

  • Fix hover logic
  • Fix click logic
  • Fix brushing logic
  • Verify pagination logic still works
  • Fix axis tick measuring
  • Fix bad cursor placement on right-side of first sm column panels. See below...
  • Fix bad cursor placement between sm rows. Fixed in 1de22e4

Future tasks

Issues

close #1484

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • 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

@nickofthyme nickofthyme added wip work in progress :heatmap Heatmap/Swimlane chart related issue :small multiples Small multiples/trellising related issues labels Jan 12, 2023
@nickofthyme nickofthyme requested a review from markov00 January 12, 2023 17:04
- move shared sm logic to common
- move sm scales to internal state
- colocate panel utils with common sm logic
- merge/simplify common types
- add heatmap sm example
- update grid sm story knobs
- Add missing files
- Update moved file path locations
.eslintrc.js Outdated Show resolved Hide resolved
  - previously providing a single input would yeild a zero-width brush area
  - this change attempts to find the next x interval value for a non-zero-width brush area
@nickofthyme
Copy link
Collaborator Author

buildkite update screenshots

@nickofthyme
Copy link
Collaborator Author

buildkite test this

@nickofthyme nickofthyme merged commit 690f568 into elastic:main Mar 10, 2023
@nickofthyme nickofthyme deleted the heatmap-sm branch March 10, 2023 23:36
nickofthyme pushed a commit that referenced this pull request Mar 21, 2023
# [55.0.0](v54.0.0...v55.0.0) (2023-03-21)

### Bug Fixes

* **docs:** lint and fix EUI breaking changes ([0d14194](0d14194))

### Features

* **heatmap:** small multiples ([#1933](#1933)) ([690f568](690f568))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:heatmap Heatmap/Swimlane chart related issue :small multiples Small multiples/trellising related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Heatmap] Small multiples
2 participants