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-plugin-chart-country-map): added support for Legend options #22356

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RoBYCoNTe
Copy link

@RoBYCoNTe RoBYCoNTe commented Dec 7, 2022

SUMMARY

Country map seems to be not very clear when you have to see and check why specific color has been chosen for an area of the map. I think legend option is mandatory to improve readability of charts.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before was not possible to configure legend, now you can use specific parameters to obtain it:
image

I've added few new config props:

  • LEGEND: allow to set position of the legend (None, Top, TopLeft, TopRight, Bottom, BottomLeft, BottomRight)
  • ORIENTATION: vertical or horizontal (like a color gradient)
  • FONT SIZE: used to render each step of the legend.

TESTING INSTRUCTIONS

Run the application and create a new chart of type Country Map, you will see new parameters that can be used to customize visualization of the chart.

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

@codecov
Copy link

codecov bot commented Dec 12, 2022

Codecov Report

Merging #22356 (74e74be) into master (ff1d29c) will decrease coverage by 0.06%.
The diff coverage is 3.79%.

❗ Current head 74e74be differs from pull request most recent head aba2fc0. Consider uploading reports for the commit aba2fc0 to get more accurate results

@@            Coverage Diff             @@
##           master   #22356      +/-   ##
==========================================
- Coverage   66.85%   66.78%   -0.07%     
==========================================
  Files        1847     1848       +1     
  Lines       70560    70638      +78     
  Branches     7737     7768      +31     
==========================================
+ Hits        47173    47176       +3     
- Misses      21380    21455      +75     
  Partials     2007     2007              
Flag Coverage Δ
javascript 53.69% <3.79%> (-0.11%) ⬇️

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

Impacted Files Coverage Δ
.../legacy-plugin-chart-country-map/src/CountryMap.js 0.00% <0.00%> (ø)
...s/legacy-plugin-chart-country-map/src/countries.ts 100.00% <ø> (ø)
...ins/legacy-plugin-chart-country-map/src/legend.tsx 0.00% <0.00%> (ø)
...acy-plugin-chart-country-map/src/transformProps.js 0.00% <ø> (ø)
...egacy-plugin-chart-country-map/src/controlPanel.ts 57.14% <60.00%> (+7.14%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

fix: removed unuseful console log message.
fix: fixed legend issues with partitioned values.
@igamat
Copy link

igamat commented Dec 29, 2023

Hi @RoBYCoNTe,
I am currently working with Superset and I definitely see this feat as a must to be included in the Country map options. However, I see there's a small issue with the legend values as in the BEFORE/AFTER SCREENSHOT image the last two value ranges are overlapping. Is that the expected behavior or is that a mistake ? In that case, how can this issue be solved ?
Thank you.

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.

None yet

2 participants