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: debug state for the heatmap chart #976

Merged
merged 9 commits into from
Mar 11, 2021
Merged

feat: debug state for the heatmap chart #976

merged 9 commits into from
Mar 11, 2021

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jan 14, 2021 •
edited
Loading

Summary

Resolves #959

  • Adds debug state support for the heatmap/swimlane chart
  • Extends DebugState interface with heatmap related data under the heatmap property
  • Adds debug action and cell selection knobs to the storybook

@darnautov darnautov added the :heatmap Heatmap/Swimlane chart related issue label Jan 14, 2021
@darnautov darnautov self-assigned this Jan 14, 2021
nickofthyme
nickofthyme reviewed Jan 14, 2021
View reviewed changes
src/chart_types/heatmap/state/selectors/get_debug_state.ts Outdated
Comment on lines 43 to 47
export type HeatmapDebugState = Pick<DebugState, 'legend'> & {
cells: CellDebug[];
selectedArea: { x: number; y: number; width: number; height: number } | null;
axes: DebugStateAxes;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

We had talked about using a single DebugState type for all chart types to simplify the API. In that case, we would add these additional keys on the DebugState as optional.

@markov00 any thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'm also for simplifying the API here if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey @nickofthyme @markov00 , I've updated the DebugState interface, please take a look at the recent changes

Base automatically changed from master to masterx February 2, 2021 23:10
Base automatically changed from masterx to master February 2, 2021 23:28
@codecov-io
Copy link

codecov-io commented Mar 10, 2021 •
edited
Loading

Codecov Report

Merging #976 (077a91b) into master (a9648a4) will increase coverage by 0.30%.
The diff coverage is 40.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #976      +/-   ##
==========================================
+ Coverage   72.47%   72.78%   +0.30%     
==========================================
  Files         366      383      +17     
  Lines       11361    11701     +340     
  Branches     2472     2524      +52     
==========================================
+ Hits         8234     8516     +282     
- Misses       3112     3162      +50     
- Partials       15       23       +8
Flag Coverage Δ
unittests 72.37% <40.54%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/chart_types/heatmap/specs/heatmap.ts 66.66% <ø> (ø)
src/components/portal/types.ts 100.00% <ø> (ø)
...es/heatmap/state/selectors/get_highlighted_area.ts 46.66% <20.00%> (-13.34%) ⬇️
...t_types/heatmap/state/selectors/get_debug_state.ts 39.13% <39.13%> (ø)
src/components/portal/tooltip_portal.tsx 89.61% <50.00%> (-3.55%) ⬇️
src/chart_types/heatmap/state/chart_state.tsx 63.76% <66.66%> (+0.53%) ⬆️
src/mocks/specs/specs.ts 81.81% <0.00%> (ø)
src/mocks/store/index.ts 100.00% <0.00%> (ø)
src/mocks/specs/index.ts 100.00% <0.00%> (ø)
src/mocks/series/index.ts 100.00% <0.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a8d016...077a91b. Read the comment docs.

@darnautov darnautov requested a review from nickofthyme March 10, 2021 13:13
nickofthyme
nickofthyme approved these changes Mar 10, 2021
View reviewed changes
Copy link
Collaborator

@nickofthyme nickofthyme left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is exactly what I was thinking. Thanks for the changes.

src/state/types.ts Outdated
Comment on lines 84 to 87
type HeatmapDebugState = {
cells: CellDebug[];
selectedArea: { x: number; y: number; width: number; height: number } | null;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏼

markov00
markov00 approved these changes Mar 11, 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.

Looks good to merge when Jenkins is green

@darnautov darnautov merged commit 2ae2bbc into elastic:master Mar 11, 2021
@darnautov darnautov deleted the swimlane-debug-data branch March 11, 2021 11:07
github-actions bot pushed a commit that referenced this pull request Mar 11, 2021
@semantic-release-bot
chore(release): 25.3.0 [skip ci]
bc1aea3 
# [25.3.0](v25.2.0...v25.3.0) (2021-03-11)

### Bug Fixes

* **brush:** force brush tool per panel ([#1071](#1071)) ([8f866fc](8f866fc)), closes [#1070](#1070)

### Features

* debug state for the heatmap chart  ([#976](#976)) ([2ae2bbc](2ae2bbc))
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 25.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Mar 11, 2021
@rshen91 rshen91 mentioned this pull request Nov 1, 2021
Closed
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@semantic-release-bot
chore(release): 25.3.0 [skip ci]
0085472 
# [25.3.0](elastic/elastic-charts@v25.2.0...v25.3.0) (2021-03-11)

### Bug Fixes

* **brush:** force brush tool per panel ([opensearch-project#1071](elastic/elastic-charts#1071)) ([61d6b7a](elastic/elastic-charts@61d6b7a)), closes [opensearch-project#1070](elastic/elastic-charts#1070)

### Features

* debug state for the heatmap chart  ([opensearch-project#976](elastic/elastic-charts#976)) ([9ba7666](elastic/elastic-charts@9ba7666))
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

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees

@darnautov darnautov

Labels
:heatmap Heatmap/Swimlane chart related issue released Issue released publicly
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Debug state] Expose debug state for the swim lane chart
4 participants
@darnautov @codecov-io @nickofthyme @markov00