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(chart): Added SriLanka country map #23338

Merged
merged 2 commits into from
Mar 18, 2023
Merged

feat(chart): Added SriLanka country map #23338

merged 2 commits into from
Mar 18, 2023

Conversation

rukshn
Copy link
Contributor

@rukshn rukshn commented Mar 13, 2023

SUMMARY

The Sri Lanka country map is not included in the chart country map plugin. This commit adds Sri Lanka map to the chart country map plugin.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

image

TESTING INSTRUCTIONS

cd superset/superset-frontend
npm run dev-server

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 Mar 13, 2023

Codecov Report

Merging #23338 (fd90bb0) into master (a8d5cb8) will increase coverage by 0.07%.
The diff coverage is 84.54%.

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

@@            Coverage Diff             @@
##           master   #23338      +/-   ##
==========================================
+ Coverage   67.51%   67.58%   +0.07%     
==========================================
  Files        1909     1907       -2     
  Lines       73465    73530      +65     
  Branches     7975     7980       +5     
==========================================
+ Hits        49601    49697      +96     
+ Misses      21816    21785      -31     
  Partials     2048     2048              
Flag Coverage Δ
javascript 53.82% <75.47%> (+0.08%) ⬆️

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

Impacted Files Coverage Δ
...rt-controls/src/shared-controls/customControls.tsx 17.39% <0.00%> (ø)
...-core/src/hooks/useChangeEffect/useChangeEffect.ts 100.00% <ø> (ø)
...hooks/useComponentDidMount/useComponentDidMount.ts 100.00% <ø> (ø)
...oks/useComponentDidUpdate/useComponentDidUpdate.ts 100.00% <ø> (ø)
...src/hooks/useElementOnScreen/useElementOnScreen.ts 100.00% <ø> (ø)
...erset-ui-core/src/hooks/usePrevious/usePrevious.ts 100.00% <ø> (ø)
...re/src/hooks/useTruncation/useCSSTextTruncation.ts 100.00% <ø> (ø)
...c/hooks/useTruncation/useChildElementTruncation.ts 100.00% <ø> (ø)
...s/legacy-plugin-chart-country-map/src/countries.ts 100.00% <ø> (ø)
...chart-echarts/src/Timeseries/Area/controlPanel.tsx 40.00% <ø> (ø)
... and 132 more

... and 1 file with indirect coverage changes

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

@rukshn rukshn changed the title feat (chart): Added SriLanka country map feat(chart): Added SriLanka country map Mar 13, 2023
@rusackas
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@rusackas Ephemeral environment spinning up at http://52.27.36.209:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@@ -174,6 +175,7 @@ export const countries = {
slovenia,
spain,
sweden,
srilanka,
Copy link
Member

Choose a reason for hiding this comment

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

The linter seems to be complaining about this PR. It might be that this is not quite in alphabetical order. You can have it (hopefully) auto-fix by running pre-commit run --all-files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes i will run it and push again.

@rusackas
Copy link
Member

The test environment isn't working for some reason. Sigh.

The pre-commit hook is complaining about formatting. I made a comment above about a hunch I have there. If you could run that and commit the changes, it should allow me to spin up another ephemeral environment for testing. 🤞

@rukshn
Copy link
Contributor Author

rukshn commented Mar 16, 2023

The test environment isn't working for some reason. Sigh.

The pre-commit hook is complaining about formatting. I made a comment above about a hunch I have there. If you could run that and commit the changes, it should allow me to spin up another ephemeral environment for testing. 🤞

I have fixed the linting issues, the tests should pass now. @rusackas Please approve the workflow tests to run.

@rukshn
Copy link
Contributor Author

rukshn commented Mar 18, 2023

@rusackas I think the PR is good to merge

@villebro villebro merged commit a5c31b2 into apache:master Mar 18, 2023
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@rusackas
Copy link
Member

I'm trying to use this now, and I can't seem to get it to work. The ISO 3166-2 codes don't seem to map to the values I'm using, whether they're provinces (LK-1 through LK-9) or Districts (LK-11 through LK-92). Can you share your sample data that's rendering on the map?

Also, I'm curious if we should change the country name from "Srilanka" to "Sri Lanka"
image

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.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 size/XXL 🚢 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants