Update BigNumber #5469

kristw · 2018-07-23T23:31:28Z

Revise the big number design.

Implement BigNumberVis as react component, with simple adaptor for transforming superset data into it. The trendline is a data-ui component.
The visual ensures that the big numbers are aligned, with or without subheader.
The number sizes vary depends on available width and presence of the trendline.
Revise visutils to use svg for text size estimation because svg can support css class and also returns height while context.measureText() only returns width and does not support css class.

Reference: Refactored from PR 4313 by @williaster
cf2c0ff

mistercrunch · 2018-07-24T01:02:57Z

Welcome on board @kristw! Please paste a screenshot. [optional] you may want to preserve the original commit to keep the attribution in place and make it possible to see the changes on top of the original PR. We'd have to be careful to Merge and not Squash and Merge to ensure it gets preserve on master. Oh and you may want to update your Github profile :)

williaster · 2018-07-24T17:41:47Z

no worries about the history preservation, I closed #4313.

I think we'll still iterate on the styling of this to make sure there's an option to set color based on the compare ratio delta.

codecov-io · 2018-07-27T21:14:21Z

Codecov Report

Merging #5469 into master will increase coverage by 0.02%.
The diff coverage is 20.28%.

@@            Coverage Diff             @@
##           master    #5469      +/-   ##
==========================================
+ Coverage   63.29%   63.31%   +0.02%     
==========================================
  Files         349      349              
  Lines       22194    22212      +18     
  Branches     2467     2473       +6     
==========================================
+ Hits        14047    14064      +17     
- Misses       8133     8134       +1     
  Partials       14       14

Impacted Files	Coverage Δ
superset/assets/src/visualizations/BigNumber.jsx	`0% <0%> (ø)`
superset/assets/src/modules/visUtils.js	`58.33% <58.33%> (-16.67%)`	⬇️
superset/models/core.py	`86.95% <0%> (-0.05%)`	⬇️
superset/views/base.py	`67.56% <0%> (ø)`	⬆️
superset/assets/src/explore/controls.jsx	`46.96% <0%> (ø)`	⬆️
superset/views/core.py	`74.19% <0%> (+0.01%)`	⬆️
superset/assets/src/visualizations/nvd3_vis.js	`8.99% <0%> (+0.03%)`	⬆️
superset/security.py	`73.09% <0%> (+0.19%)`	⬆️
...rc/explore/components/controls/AnnotationLayer.jsx	`23.35% <0%> (+0.69%)`	⬆️

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 4bf69a7...c935050. Read the comment docs.

williaster

This looks great to me overall, nice improvements! This will look so much better 😍

One question: it looks like BigNumber supports a className, what are your thoughts on varying the className based on the trend (+/-, or 0)? That way you could have custom css hooks to change color based on this.

Will get the react-15-compatible @data-ui/xy-chart version out ASAP.

williaster · 2018-07-27T21:11:15Z

superset/assets/src/visualizations/BigNumber.jsx

+  };
+}
+
+const propTypes = {


future lint rules will require all PropTypes that aren't required to have defaults, could add those now.

sure. will take care of that.

williaster · 2018-07-27T21:13:54Z

superset/assets/src/visualizations/BigNumber.jsx

+BigNumberVis.propTypes = propTypes;
+BigNumberVis.defaultProps = defaultProps;
+
+function adaptor(slice, payload) {


❤️ the term adaptor for this + code style here

williaster · 2018-07-27T21:15:07Z

superset/assets/src/visualizations/big_number.css

-  stroke-width: 1;
+.big_number_header_line {
+  position: relative;
+  font-family: -apple-system,BlinkMacSystemFont,Segoe UI,Roboto,Oxygen,Ubuntu,Cantarell,Open Sans,Helvetica Neue,sans-serif;


do you need to define font-family more than once or can you inherit from the container? or is this necessary for proper size-detection?

Necessary for proper size detection. The getTextDimension function accept className and create svg text with that class.

const fontSize = computeMaxFontSize({ text, maxWidth: width, maxHeight, className: 'big_number_header_line', });

Otherwise the util function will have to create same nesting structure (with outer container have .big_number) or be able to accept parent DOM node (which does not exist yet).

I understand this looks annoyingly redundant, but don't have a good work around yet. Unless all body use same font family.

makes sense 👍and I agree no obvious work around. We could possibly add a comment in the css that removing it would potentially break proper sizing.

I thought about this again and made the util function accept container DOM element, therefore got rid of the font family.

kristw · 2018-07-27T21:34:14Z

@williaster For clarification, will the positive/negative cssClass be for subheader? I think that should be doable. Will find a clean way to do it.

williaster · 2018-07-27T21:37:08Z

yeah I think that'd make the most sense. could possibly add it for the trend line, but actually they could probably change that if they really wanted if the header had the class.

williaster · 2018-07-31T22:05:33Z

superset/assets/package.json

@@ -45,6 +45,7 @@
  "dependencies": {
    "@data-ui/event-flow": "^0.0.54",
    "@data-ui/sparkline": "^0.0.54",
+    "@data-ui/xy-chart": "^0.0.60-alpha.1.0",


0.0.60 is out now FYI

support toggling trend line experiment using svg to measure size instead of canvas refactor BigNumber with chart sticked to the bottom made header line stick to bottom and css adjustment remove commented code fix svg text size estimation remove unused code and round dimensions handle missing getbbox remove vx/text from dependency ensure svg deletion after measurement add comment to css file. Add `positive` and `negative` class based on diff. Add default props. rename variable accept container as argument and remove redundant font family refactor visutils for consistent api update xy-chart update xy-chart to alpha1.0 fix pointseries remove points update yarn.lock [4c9c01d7] update xy-chart version remove unused import

kristw · 2018-08-02T17:08:54Z

@williaster ready to merge

williaster · 2018-08-02T17:34:32Z

solid 🙌 🚢

mistercrunch · 2018-08-02T17:54:33Z

support toggling trend line experiment using svg to measure size instead of canvas refactor BigNumber with chart sticked to the bottom made header line stick to bottom and css adjustment remove commented code fix svg text size estimation remove unused code and round dimensions handle missing getbbox remove vx/text from dependency ensure svg deletion after measurement add comment to css file. Add `positive` and `negative` class based on diff. Add default props. rename variable accept container as argument and remove redundant font family refactor visutils for consistent api update xy-chart update xy-chart to alpha1.0 fix pointseries remove points update yarn.lock [4c9c01d7] update xy-chart version remove unused import

williaster mentioned this pull request Jul 24, 2018

[big number] improve visual aesthetics of big number visualization #4313

Closed

2 tasks

kristw force-pushed the kristw/big-number branch from 96982c0 to b73f33e Compare July 27, 2018 20:44

williaster reviewed Jul 27, 2018

View reviewed changes

williaster reviewed Jul 31, 2018

View reviewed changes

kristw force-pushed the kristw/big-number branch 2 times, most recently from 5c8bd19 to 98ab56a Compare August 2, 2018 05:35

kristw force-pushed the kristw/big-number branch from 98ab56a to c935050 Compare August 2, 2018 06:05

williaster merged commit 1b9e5d4 into apache:master Aug 2, 2018

kristw mentioned this pull request Aug 9, 2018

Cherry-pick big number changes and code cleanup airbnb/superset-fork#76

Merged

kristw deleted the kristw/big-number branch August 13, 2018 20:59

kristw mentioned this pull request Aug 28, 2018

[SIP-5] [Embeddable Charts] Decouple charts from slice and formData #5680

Closed

conglei mentioned this pull request Sep 11, 2018

[SIP-6] [Embeddable Charts] Migrate visualization-specific code to js plugin #5692

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update BigNumber #5469

Update BigNumber #5469

kristw commented Jul 23, 2018 •

edited

Loading

mistercrunch commented Jul 24, 2018

williaster commented Jul 24, 2018

codecov-io commented Jul 27, 2018 •

edited

Loading

williaster left a comment

williaster Jul 27, 2018

kristw Jul 27, 2018

williaster Jul 27, 2018

williaster Jul 27, 2018

kristw Jul 27, 2018 •

edited

Loading

williaster Jul 27, 2018

kristw Jul 27, 2018

kristw commented Jul 27, 2018

williaster commented Jul 27, 2018

williaster Jul 31, 2018

kristw commented Aug 2, 2018

williaster commented Aug 2, 2018

mistercrunch commented Aug 2, 2018

Update BigNumber #5469

Update BigNumber #5469

Conversation

kristw commented Jul 23, 2018 • edited Loading

mistercrunch commented Jul 24, 2018

williaster commented Jul 24, 2018

codecov-io commented Jul 27, 2018 • edited Loading

Codecov Report

williaster left a comment

Choose a reason for hiding this comment

williaster Jul 27, 2018

Choose a reason for hiding this comment

kristw Jul 27, 2018

Choose a reason for hiding this comment

williaster Jul 27, 2018

Choose a reason for hiding this comment

williaster Jul 27, 2018

Choose a reason for hiding this comment

kristw Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

williaster Jul 27, 2018

Choose a reason for hiding this comment

kristw Jul 27, 2018

Choose a reason for hiding this comment

kristw commented Jul 27, 2018

williaster commented Jul 27, 2018

williaster Jul 31, 2018

Choose a reason for hiding this comment

kristw commented Aug 2, 2018

williaster commented Aug 2, 2018

mistercrunch commented Aug 2, 2018

kristw commented Jul 23, 2018 •

edited

Loading

codecov-io commented Jul 27, 2018 •

edited

Loading

kristw Jul 27, 2018 •

edited

Loading