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(legacy-preset-chart-deckgl): Add ,.1f and ,.2f value formats to deckgl charts #18945

Merged
merged 2 commits into from
Feb 28, 2022

Conversation

kgabryje
Copy link
Member

SUMMARY

This PR adds 2 new formattings to Legend Format control in deckgl charts.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

image

TESTING INSTRUCTIONS

Verify that ,.1f and ,.2f legend formattings are available and work correctly

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

https://app.shortcut.com/preset/story/37656/add-1f-and-2f-to-the-legend-format-option-for-deck-gl-charts

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #18945 (a173f74) into master (2cb3635) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #18945   +/-   ##
=======================================
  Coverage   66.38%   66.38%           
=======================================
  Files        1640     1641    +1     
  Lines       63514    63522    +8     
  Branches     6418     6421    +3     
=======================================
+ Hits        42164    42170    +6     
- Misses      19690    19691    +1     
- Partials     1660     1661    +1     
Flag Coverage Δ
javascript 51.00% <ø> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...perset-ui-chart-controls/src/utils/D3Formatting.ts 100.00% <ø> (ø)
...reset-chart-deckgl/src/utilities/Shared_DeckGL.jsx 84.21% <ø> (ø)
...acy-preset-chart-deckgl/src/utilities/controls.jsx 11.11% <ø> (-8.89%) ⬇️
...d/src/SqlLab/components/SqlEditorLeftBar/index.tsx 56.66% <0.00%> (-0.48%) ⬇️
superset-frontend/src/views/components/Menu.tsx 56.41% <0.00%> (ø)
superset-frontend/src/views/components/SubMenu.tsx 89.36% <0.00%> (ø)
...uperset-frontend/src/components/Dropdown/index.tsx 100.00% <0.00%> (ø)
...perset-frontend/src/views/CRUD/chart/ChartCard.tsx 47.61% <0.00%> (ø)
...et-frontend/src/dashboard/components/SaveModal.tsx 41.93% <0.00%> (ø)
...t-frontend/src/views/CRUD/welcome/SavedQueries.tsx 52.38% <0.00%> (ø)
... and 29 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 2cb3635...a173f74. Read the comment docs.

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

In order to provide more format for this chart, we just add freeForm: true for format control in Deckgl.

@kgabryje
Copy link
Member Author

Thanks for pointing that out @zhaoyongjie. @yousoph @rusackas should we add those formatters in other charts as well? Should we quick and simple.

@yousoph
Copy link
Member

yousoph commented Feb 25, 2022

Thanks for pointing that out, @zhaoyongjie !
@kgabryje I think it makes sense to add the freeform formatter to deck.gl (and any other chart types it's missing from)

@kgabryje
Copy link
Member Author

@zhaoyongjie @yousoph Added ,.1f and ,.2f to D3_FORMAT_OPTIONS globally, added freeForm to deckgl, also removed duplicated code in deckgl plugins.

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement!

@kgabryje kgabryje merged commit c56dc8e into apache:master Feb 28, 2022
villebro pushed a commit that referenced this pull request Apr 3, 2022
…deckgl charts (#18945)

* Add ,.1f and ,.2f value formats to deckgl charts

* Remove duplicated code

(cherry picked from commit c56dc8e)
@mistercrunch mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels lts-v1 size/S 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants