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.

Sign up for GitHub

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

Jump to bottom

update dataviz demo pages #2223

Merged
merged 5 commits into from
Apr 16, 2021
Merged

update dataviz demo pages #2223

merged 5 commits into from
Apr 16, 2021

Conversation

theiliad
Copy link
Member

@theiliad theiliad commented Apr 13, 2021 •
edited
Loading

  • Changes basic charts to simple
  • Changes advanced charts to complex
  • Updates demos
  • Adds the 10 more demos cards to each demo group

TODO:

  • Separate out the stacked area demos in @carbon/charts

@theiliad theiliad requested a review from a team April 13, 2021 00:09
@theiliad theiliad requested a review from a team as a code owner April 13, 2021 00:09
@vercel
Copy link

vercel bot commented Apr 13, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-design-system/carbondesignsystem/EjFPS13H1Gke2ybyeELb4fBMyUa9
✅ Preview: https://carbondesignsy-git-fork-theiliad-dataviz-demo-reorg-carb-ce0dd0.vercel.app

@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2021 •
edited
Loading

DCO Assistant Lite bot All contributors have signed the DCO.

@theiliad
Copy link
Member Author

Closes #2175

@mjabbink
Copy link
Contributor

mjabbink commented Apr 13, 2021 •
edited
Loading

@theiliad This is awesome. So much better for our users!

— Noticed Storybook icon missing in first resource card
Screen Shot 2021-04-12 at 8 07 50 PM

— Some spacing issue. Image and text too close. @jeanservaas will know spec but the Gatsby should not being doing this I don’t think. Header should be farther away from the text as well.
Screen Shot 2021-04-12 at 8 07 50 PM

@mjabbink
Copy link
Contributor

Should Sparkline be shown on an axis grid?

@mjabbink
Copy link
Contributor

Will Bullet chart be in Complex or Simple? I think we decided in Simple no?

@mjabbink
Copy link
Contributor

@theiliad Can we change these situation in storybook site so we use either a em dash between these situations:

(time series—dense data)

or do it like

(time series/dense data)

@jeanservaas what do you think? I guess if we can keep hyphens but remove space but I think the above two options are better.

@theiliad
Copy link
Member Author

Should Sparkline be shown on an axis grid?

no sparkline doesn't show up on axes, at least that's the use case it was designed for.

Here's what it looks like with all of the elements visible
image

we can add this demo later if we need to

@theiliad
Copy link
Member Author

Will Bullet chart be in Complex or Simple? I think we decided in Simple no?

sure we can do that, once it's out

@theiliad theiliad requested a review from vpicone as a code owner April 13, 2021 17:16
@theiliad
Copy link
Member Author

we're good to go here, please squash & merge

@jeanservaas we're going to need thumbnails soon for bullet & circle pack

@theiliad
Copy link
Member Author

theiliad commented Apr 13, 2021 •
edited
Loading

@jeanservaas we're going to need thumbnails soon for bullet & circle pack

I guess we can merge this without those icons bc we actually need icons for combo, lollipop, sparkline, circle pack, boxplot, wordcloud, bullet & floating bar. Will have to be a separate PR in a week or 2

@mjabbink
Copy link
Contributor

@theiliad I like the axis elements in place. That’s where and how it would be utilized. Looks strange to me without it. @jeanservaas what are your thought on Sparline not having the axis structure.

@mjabbink
Copy link
Contributor

@theiliad I also think you can merge before @jeanservaas get’s the thumbnails. @jeanservaas Do you agree?

@tw15egan
Copy link
Collaborator

@theiliad can you sign the DCO?

@theiliad
Copy link
Member Author

I have read the DCO document and I hereby sign the DCO.

I have read the DCO document and I hereby sign the DCO.

@theiliad
Copy link
Member Author

I have read the DCO document and I hereby sign the DCO.

@theiliad
Copy link
Member Author

I have read the DCO document and I hereby sign the DCO.

@tw15egan I don't think this is working

@tw15egan
Copy link
Collaborator

recheck

@theiliad
Copy link
Member Author

Good thanks

tw15egan
tw15egan approved these changes Apr 13, 2021
View reviewed changes
Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

jillianhowarth
jillianhowarth approved these changes Apr 14, 2021
View reviewed changes
Copy link
Contributor

@jillianhowarth jillianhowarth left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@kodiakhq
Copy link
Contributor

kodiakhq bot commented Apr 15, 2021

This PR currently has a merge conflict. Please resolve this and then re-add the status: ready to merge 🎉 label.

@tw15egan tw15egan requested a review from vpicone April 15, 2021 20:20
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Apr 16, 2021

This PR currently has a merge conflict. Please resolve this and then re-add the status: ready to merge 🎉 label.

@mjabbink
Copy link
Contributor

👍

@mjabbink
Copy link
Contributor

Page looks great. Thank you @theiliad! This is a huge improvement for anyone coming to this page.

@tw15egan tw15egan force-pushed the dataviz-demo-reorg branch from f65a19f to 0d028aa Compare April 16, 2021 19:56
vpicone
vpicone approved these changes Apr 16, 2021
View reviewed changes
Copy link
Contributor

@vpicone vpicone left a comment

Choose a reason for hiding this comment

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

rewrites look good

@kodiakhq kodiakhq bot merged commit afe9c36 into carbon-design-system:main Apr 16, 2021
@theiliad theiliad deleted the dataviz-demo-reorg branch April 16, 2021 21:16
@theiliad theiliad mentioned this pull request Apr 27, 2021
Closed
2 tasks
natashadecoste pushed a commit to natashadecoste/carbon-website that referenced this pull request May 19, 2021
@theiliad @natashadecoste
update dataviz demo pages (carbon-design-system#2223)
fa02dff 
* update dataviz demo pages

* update chart types page links

* final touches

* add boxplot & wordcloud images

* add lollipop to demos and move wordcloud into comparisons
natashadecoste pushed a commit to natashadecoste/carbon-website that referenced this pull request May 19, 2021
@theiliad @natashadecoste
update dataviz demo pages (carbon-design-system#2223)
f155f91 
* update dataviz demo pages

* update chart types page links

* final touches

* add boxplot & wordcloud images

* add lollipop to demos and move wordcloud into comparisons
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jillianhowarth jillianhowarth jillianhowarth approved these changes

@tw15egan tw15egan tw15egan approved these changes

@vpicone vpicone vpicone approved these changes

@joshblack joshblack joshblack approved these changes

@andreancardona andreancardona Awaiting requested review from andreancardona

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@theiliad @mjabbink @tw15egan @joshblack @vpicone @jillianhowarth