search

kyndryl-design-system / shidoka-charts

Shidoka charts based on Chart.js
https://shidoka-charts.netlify.app
MIT License
1 stars 0 forks source link

feat: Changes on geo charts based on UX suggestions #4

Closed maulik-thaker1 closed 12 months ago

maulik-thaker1 commented 1 year ago

Summary

  1. Added datelabels to US choropleth and Bubble map
  2. Flatten world choropleth chart & removed grids
  3. Added border width and font size to all geo charts
  4. Added zoom plugin for future enhancement
netlify[bot] commented 1 year ago

Deploy Preview for shidoka-charts ready!

Name Link
Latest commit a11c72a08e82152bab61115a884b59944b6891f1
Latest deploy log https://app.netlify.com/sites/shidoka-charts/deploys/654539744a151600082ead45
Deploy Preview https://deploy-preview-4--shidoka-charts.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

maulik-thaker1 commented 12 months ago
  1. Added datalabels to US choropleth and Bubble map
  2. Flatten world choropleth chart & removed grids
  3. Added border width and font size to all geo charts
  4. Added zoom plugin for future enhancement
  1. The datalabels look weird on the Choropleth to me. Also, the value being shown is the scaled value out of 10, not the actual value of the data used to create the chart. Is that really what we intend to show?
  2. Looks cleaner without the globe lines, but kind of stretched.
  3. You only added this in the story. If you want it to be a default configuration, you need to add it to the config/chartTypes/json file. Same goes for datalabel changes.
  4. I would remove this as a dependency if it's not being used at all.

Remember, US and World Choropleth are the same chart type, and share the same default config. If datalabels is not going into the default config JSON, it should be a separate story.

  1. Yes. As of now for internal demo purpose tomorrow. Datalabel showing value which we get on hovered part.
  2. Yeah kind of but UX wants to keep this.
  3. ok Do you mean inside chartArgTypes.js file? I dont think is default one as world choropleth showing label very oddly
  4. ok. I 'll remove zoom plugin as of now.

here's how datalable shows on world choropleth

Screenshot 2023-11-02 at 8 24 01 PM

Note: As per Chandan's point of view she told for now we gonna demo this and get feedback from team later on we can explore more possibilities if possible.

maulik-thaker1 commented 12 months ago

Removed Zoom plugin from package.json file.

brian-patrick-3 commented 12 months ago 
2. Yeah kind of but UX wants to keep this.

3. ok Do you mean inside **chartArgTypes.js** file?
   I dont think is default one as world choropleth showing label very oddly
  1. We might want to change the labels and the tooltip to show the actual value of the data, rather than the scaled value.
  2. Try projection "equalEarth" or "naturalEarth1" (this should actually probably be the default instead of "albersUsa", since World maps will be more common than US) and showOutline: false.
  3. These files are the chartType-specific default configs.
maulik-thaker1 commented 12 months ago

2. naturalEarth1

2. Yeah kind of but UX wants to keep this.

3. ok Do you mean inside **chartArgTypes.js** file?
   I dont think is default one as world choropleth showing label very oddly
  1. We might want to change the labels and the tooltip to show the actual value of the data, rather than the scaled value.
  2. Try projection "equalEarth" or "naturalEarth1" (this should actually probably be the default instead of "albersUsa", since World maps will be more common than US) and showOutline: false.
  3. These files are the chartType-specific default configs.
  1. there's no actual value in dataset I guess. We are generating random value by giving value: Math.random() * 10 kind of random value generation

for example : random value generate for Nevada is 2.335 and on hover, its shows on tooltip and also in datalable

Screenshot 2023-11-02 at 9 22 26 PM
  1. If we do projection: "equalEarth" and "naturalEarth1" then map is not gonna flatten. For flatten world map, we need to give 'equirectangular' only. if we don't want flatten map then I can keep equalEarth and naturalEarth1. Should we restrict flatten for Global map?
  2. ok got it. Are they over written at stoybook ?
brian-patrick-3 commented 12 months ago 
1. there's no actual value in dataset I guess. We are generating random value by giving `value: Math.random() * 10` kind of random value generation

2. If we do projection: "`equalEarth`" and "`naturalEarth1`" then map is not gonna flatten. For flatten world map, we need to give '`equirectangular`' only. if we don't want flatten map then I can keep **equalEarth** and **naturalEarth1**. Should we restrict flatten for Global map?

3. ok got it. Are they over written at stoybook ?
  1. For example purposes the values are random, but these will be real values in real maps. Showing the scaled value in datalabels does not really provide any value. I think remove datalabels entirely from Choropleth, only use them on BubbleMap to show City names or similar (like the geo plugin example).
  2. We can't restrict anything, we are just choosing the default. Carbon Charts for example uses naturalEarth1 as the default, which looks the best in my opinion. It looks more flat with the outline removed.
  3. Yes. The load order for options configs is:
    1. globalOptions.js
    2. globalOptionsRadial.js/NonRadial.js
    3. chartType/{chartType}.js
    4. options passed to kd-chart component
maulik-thaker1 commented 12 months ago

2. naturalEarth1

1. there's no actual value in dataset I guess. We are generating random value by giving `value: Math.random() * 10` kind of random value generation

2. If we do projection: "`equalEarth`" and "`naturalEarth1`" then map is not gonna flatten. For flatten world map, we need to give '`equirectangular`' only. if we don't want flatten map then I can keep **equalEarth** and **naturalEarth1**. Should we restrict flatten for Global map?

3. ok got it. Are they over written at stoybook ?
  1. For example purposes the values are random, but these will be real values in real maps. Showing the scaled value in datalabels does not really provide any value. I think remove datalabels entirely from Choropleth, only use them on BubbleMap to show City names or similar (like the geo plugin example).
  2. We can't restrict anything, we are just choosing the default. Carbon Charts for example uses naturalEarth1 as the default, which looks the best in my opinion. It looks more flat with the outline removed.

  3. Yes. The load order for options configs is:

    1. globalOptions.js
    2. globalOptionsRadial.js/NonRadial.js
    3. chartType/{chartType}.js
    4. options passed to kd-chart component
  1. Ok gotcha. I'm removing dataLables from choropleth and only apply on Bubblemap
  2. Ok. I made it naturalEarth1 and it's looks like follow with outline removed :
Screenshot 2023-11-02 at 11 13 32 PM

I pushed code with these changes. Can you approve now ?

brian-patrick-3 commented 12 months ago

I pushed code with these changes. Can you approve now ?

  1. The default config changes still need to be moved into the chartType JS config files. Use "World" as the default, and make it the first story.
  2. Bubble Map datalabels font size seems like it may bee too small for accessibility.
  3. Stories are still under "Proof of Concept", let's change that if these are ready.
maulik-thaker1 commented 12 months ago

I pushed code with these changes. Can you approve now ?

  1. The default config changes still need to be moved into the chartType JS config files. Use "World" as the default, and make it the first story.
  2. Bubble Map datalabels font size seems like it may bee too small for accessibility.
  3. Stories are still under "Proof of Concept", let's change that if these are ready.
  1. Done. Added datalable plugin into bubblemap default config file named : bubbleMap.js and hoverBorderWidth into : choropleth.js & bubbleMap.js Also made World choropleth as first story.
  2. Done. Changed to 12 for better visibility.
  3. What should be the name? Third-Party Charts or Others? As on welcome page it's named Third-party-charts so I named accordingly.
brian-patrick-3 commented 12 months ago 
3. What should be the name? **Third-Party Charts** or **Others**? As on welcome page it's named **Third-party-charts** so I named accordingly.

Third-Party Charts is a good distinction. However, Tree Map is not ready yet as far as I know, and should remain under Proof of Concept.

Also, the Choropleth defaults (World) for projection, showOutline, etc still need to be moved to the chartType js config file. Then you'll have to adjust the options for the US and World stories.

maulik-thaker1 commented 12 months ago 
3. What should be the name? **Third-Party Charts** or **Others**? As on welcome page it's named **Third-party-charts** so I named accordingly.

Third-Party Charts is a good distinction. However, Tree Map is not ready yet as far as I know, and should remain under Proof of Concept.

Also, the Choropleth defaults (World) for projection, showOutline, etc still need to be moved to the chartType js config file. Then you'll have to adjust the options for the US and World stories.

We didn't touched much to Treemap yet as no specific design change and on demo also no one mentioned to extend this. So, I am considering maybe it's done. Maybe enhancement can open more explorations on this or later phase I guess.

oh I guess showOutline and showGraticule is only remaining. I moved it to chartType.js. Also adjusted storybook options to US and World stories.

[A] US options:

Screenshot 2023-11-03 at 8 39 33 PM

[B] World options:

Screenshot 2023-11-03 at 8 40 53 PM
brian-patrick-3 commented 12 months ago

Tree Map is still Proof of Concept, since design has not touched it yet.

You forgot to change the projection default to naturalEarth1 in the config. Once that is done, you can remove any default options from the World story.

maulik-thaker1 commented 12 months ago

Tree Map is still Proof of Concept, since design has not touched it yet.

You forgot to change the projection default to naturalEarth1 in the config. Once that is done, you can remove any default options from the World story.

I am waiting for confirmation for TreeMap is still POC or not. Apart from that I removed default options from World story & added projection in config.

maulik-thaker1 commented 12 months ago

I merged this as I no one replies for Treemap. If anything coming then we'll look do in Prague release.

brian-patrick-3 commented 12 months ago

:tada: This PR is included in version 1.0.0-beta.15 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

brian-patrick-3 commented 11 months ago

:tada: This PR is included in version 1.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: