Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: Add types and type guards for encodeable #201

Merged
merged 5 commits into from
Aug 14, 2019
Merged

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Aug 13, 2019

🏆 Enhancements

Define encoding config for a visualization

  • Each visualization has one or more channels that can be encoded, such as x, y, color, stroke, fill. These channels configurations should be consistent/sharable across all visualizations. For example, x and y usually have scales and axes. Scales have domain, range. Axes have label, format. Color can support categorical, sequential or fixed color. All channels can support fixed value, field name in the data, or sometimes aggregates (haven't implemented the aggregate yet).
  • Each encoding channel config is based on vega-lite encoding grammar. (See line chart example, vega-lite encoding documentation)
  • Only adopt a small subset of vega-lite encoding grammar for now as we don’t have enough time (and may not intend) to provide 100% feature of vega-lite.

🏠 Internal

  • The package is currently marked as private until completion.

@kristw kristw requested a review from a team as a code owner August 13, 2019 01:10
@kristw kristw added the WIP Work in progress label Aug 13, 2019
@codecov
Copy link

codecov bot commented Aug 13, 2019

Codecov Report

Merging #201 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage   99.91%   99.92%   +<.01%     
==========================================
  Files         102      107       +5     
  Lines        1222     1252      +30     
  Branches      293      309      +16     
==========================================
+ Hits         1221     1251      +30     
  Partials        1        1
Impacted Files Coverage Δ
...ages/superset-ui-encodeable/src/utils/isEnabled.ts 100% <100%> (ø)
...ges/superset-ui-encodeable/src/utils/isDisabled.ts 100% <100%> (ø)
...ages/superset-ui-encodeable/src/typeGuards/Base.ts 100% <100%> (ø)
...uperset-ui-encodeable/src/typeGuards/ChannelDef.ts 100% <100%> (ø)
...kages/superset-ui-encodeable/src/utils/identity.ts 100% <100%> (ø)
...ckages/superset-ui-chart/src/models/ChartPlugin.ts 100% <0%> (ø) ⬆️
... and 1 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 920d389...b06e9eb. Read the comment docs.

@netlify
Copy link

netlify bot commented Aug 13, 2019

Deploy preview for superset-ui ready!

Built with commit cc9c369

https://deploy-preview-201--superset-ui.netlify.com

@netlify
Copy link

netlify bot commented Aug 13, 2019

Deploy preview for superset-ui ready!

Built with commit b06e9eb

https://deploy-preview-201--superset-ui.netlify.com

@kristw kristw added reviewable Ready for review and removed WIP Work in progress labels Aug 13, 2019
Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

LGTM overall! had a couple of small comments. Thanks for refactoring this to a new package!

@@ -0,0 +1,2 @@
const x = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was templated. Now removed.

| TextFieldDef;

/** Channel definitions that are not constant value */
export type NonValueDef<Output extends Value = Value> = Exclude<
Copy link
Contributor

Choose a reason for hiding this comment

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

NonConstantValueDef for more clear naming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is ValueDef and this one is the opposite of it.

@@ -0,0 +1,6 @@
// Types directly imported from vega-lite

export { ValueDef, Value } from 'vega-lite/build/src/channeldef';
Copy link
Contributor

Choose a reason for hiding this comment

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

types/Vegalite.ts is more clear to me when seeing these imports elsewhere

@@ -0,0 +1,5 @@
describe('My Test', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

also not sure if you still need this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed.

@kristw kristw force-pushed the kristw--encodeable branch from 9c06c34 to b06e9eb Compare August 14, 2019 17:37
@kristw kristw merged commit 6cff25f into master Aug 14, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--encodeable branch August 14, 2019 18:45
kristw pushed a commit that referenced this pull request Apr 17, 2020
* NPM link docs (hat tip to Krist)

* formatting changes in prior commit
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewable Ready for review size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants