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

refactor: decouple legend and tooltip phase 1 #491

Merged
merged 6 commits into from
Feb 14, 2020

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Dec 9, 2019

Summary

This is the first phase to clean and decouple the current code for legend and tooltip from the XYChart dependency.
In this PR I've just moved out 3 types Rotation, Datum and Position from the XYChart to the main src/utils/commons.ts and I've also moved 1 level up the scales folder

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • [ ] This was checked for cross-browser compatibility, including a check against IE11
  • [ ] Proper documentation or storybook story was added for features that require explanation or tutorials
  • Unit tests were updated or added to match the most common scenarios
  • Each commit follows the convention

Copy link
Contributor

@monfera monfera 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 so far, I only have some naming suggestions

src/utils/commons.ts Show resolved Hide resolved
src/utils/commons.ts Show resolved Hide resolved
src/utils/commons.ts Show resolved Hide resolved
src/utils/commons.ts Show resolved Hide resolved
@markov00 markov00 mentioned this pull request Dec 13, 2019
4 tasks
@markov00 markov00 force-pushed the 2019-12-06_decouple branch from dca75da to e31cf93 Compare February 13, 2020 12:00
@codecov-io
Copy link

codecov-io commented Feb 13, 2020

Codecov Report

Merging #491 into master will decrease coverage by 0.28%.
The diff coverage is 80.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #491      +/-   ##
==========================================
- Coverage   71.78%   71.49%   -0.29%     
==========================================
  Files         208      208              
  Lines        6216     6119      -97     
  Branches     1199     1169      -30     
==========================================
- Hits         4462     4375      -87     
+ Misses       1738     1727      -11     
- Partials       16       17       +1
Impacted Files Coverage Δ
...es/partition_chart/layout/types/viewmodel_types.ts 100% <ø> (ø) ⬆️
...types/partition_chart/layout/types/config_types.ts 100% <ø> (ø) ⬆️
.../chart_types/partition_chart/layout/types/types.ts 100% <ø> (ø) ⬆️
src/chart_types/xy_chart/tooltip/tooltip.ts 89.65% <100%> (+3.94%) ⬆️
src/state/actions/specs.ts 93.33% <100%> (-1.67%) ⬇️
src/specs/specs_parser.tsx 100% <100%> (ø) ⬆️
...pes/xy_chart/state/selectors/is_tooltip_visible.ts 88.88% <100%> (ø) ⬆️
...chart/state/selectors/get_legend_tooltip_values.ts 100% <100%> (ø) ⬆️
.../selectors/get_tooltip_values_highlighted_geoms.ts 92.42% <100%> (-1.46%) ⬇️
...rt/state/selectors/get_annotation_tooltip_state.ts 61.53% <50%> (-2.75%) ⬇️
... and 8 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 4e445a3...9a2dd99. Read the comment docs.

@markov00 markov00 merged commit 2242fca into elastic:master Feb 14, 2020
@markov00 markov00 deleted the 2019-12-06_decouple branch February 14, 2020 11:44
markov00 added a commit that referenced this pull request Feb 21, 2020
Move `Datum`, `Rotation`, `Position` and `Color` to `utils/commons`. Decouple legend from axis position method and move the `scales` to `utils/scales`.
markov00 added a commit that referenced this pull request Mar 2, 2020
Move `Datum`, `Rotation`, `Position` and `Color` to `utils/commons`. Decouple legend from axis position method and move the `scales` to `utils/scales`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants