Skip to content

Website | Gallery: Donut Example #38#367

Merged
reb-dev merged 1 commit into
mainfrom
qian/add-donut-#38
Apr 12, 2024
Merged

Website | Gallery: Donut Example #38#367
reb-dev merged 1 commit into
mainfrom
qian/add-donut-#38

Conversation

@lee00678

Copy link
Copy Markdown
Collaborator

Add a gallery example which showcases the use of a basic donut chart. (issue)

  • Create a folder in shared/examples directory called basic-donut-chart with all the relevant files
  • Find dataset and create example in basic-donut-chart.tsx

Add code for other frameworks:

  • TypeScript
  • Angular
  • Svelte
  • Vue

And then add screenshots to _shared/previews:

  • basic-donut-chart.png
  • basic-donut-chart-dark.png

@lee00678
lee00678 requested a review from reb-dev April 10, 2024 20:20
@rokotyan

Copy link
Copy Markdown
Contributor

Great job @lee00678!

I'm a little concerned about the visual aspects of the new example because it's inconsistent with other examples and our Unovis brand. Specifically, the colors and the capitalized text in the legend.
image

@Dovlash Vlad, can you please take a look and give us your comments. Here's the Gallery section on our website if you need a link: https://unovis.dev/gallery. (Vlad is leading our data visualization design activities at F5)

@rokotyan

Copy link
Copy Markdown
Contributor

@reb-dev It might be a good time to enforce the Commit Lint formats: #300

@lee00678
lee00678 force-pushed the qian/add-donut-#38 branch from abc0227 to 9d2770f Compare April 10, 2024 21:32
@lee00678

Copy link
Copy Markdown
Collaborator Author

Great job @lee00678!

I'm a little concerned about the visual aspects of the new example because it's inconsistent with other examples and our Unovis brand. Specifically, the colors and the capitalized text in the legend.

I'm happy to make any changes! Also nice to "meet" both of you @rokotyan and @Dovlash. Yes, I used D3's color theme as I ran out of the default colors for donut chart. I can probably look into how to get unovis' color theme rendered, and I will update the legend as well.

@rokotyan

rokotyan commented Apr 10, 2024

Copy link
Copy Markdown
Contributor

Nice to meet you too @lee00678!

I see, I think instead of showing so many slices, we can merge smaller ones into a big "Others" one.
Here are two great posts about Pie Charts and Colors in data vis:

@lee00678
lee00678 force-pushed the qian/add-donut-#38 branch from 9d2770f to 815292f Compare April 11, 2024 17:33
@lee00678

lee00678 commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator Author

I see, I think instead of showing so many slices, we can merge smaller ones into a big "Others" one. Here are two great posts about Pie Charts and Colors in data vis:

I like the idea of combining the smaller categories. Made an update on that.

@rokotyan

Copy link
Copy Markdown
Contributor

Nice, thanks for the update!

@rokotyan

Copy link
Copy Markdown
Contributor

@lee00678 Btw, if you use Discord, we've a space there which we're planning to make public soon. Feel free to join: https://discord.gg/zATXusWgCP

@lee00678
lee00678 force-pushed the qian/add-donut-#38 branch from 815292f to e0fb1d5 Compare April 11, 2024 19:21
@lee00678 lee00678 self-assigned this Apr 11, 2024

@reb-dev reb-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good @lee00678! I left one comment, I think you'll need to rebase on main to address it

],
},
{
title: 'Donut Charts',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of making a new category, I would add it as the first entry to "Circular Charts"

@lee00678
lee00678 force-pushed the qian/add-donut-#38 branch from e0fb1d5 to 1ceb62a Compare April 12, 2024 02:46
@github-actions

github-actions Bot commented Apr 12, 2024

Copy link
Copy Markdown

CLA Assistant Lite bot ✅ All required contributors have signed the F5 CLA for this PR. Thank you!

@lee00678

Copy link
Copy Markdown
Collaborator Author

CLA Assistant Lite bot: 🎉 Thank you for your contribution. It appears you have not yet signed the F5 Contributor License Agreement (CLA), which is required for your changes to be incorporated into an F5 project. Please kindly read the F5 CLA and comment the following to agree:

I have hereby read the F5 CLA and agree to its terms

Qian Liu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.You can retrigger this bot by commenting recheck in this Pull Request

Agree

@lee00678
lee00678 force-pushed the qian/add-donut-#38 branch from 1ceb62a to c9e210e Compare April 12, 2024 02:57

@reb-dev reb-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @lee00678! Merging.

@reb-dev
reb-dev merged commit ba9096a into main Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants