Codesandbox examples for charting demos #987
Conversation
…te into codesandbox-dataviz
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/carbon-design-system/carbon-website/mf8fm2kew |
|
Deploy preview for carbon-website failed. Built with commit c4c8869 https://app.netlify.com/sites/carbon-website/deploys/5e8f8efc021d44000718d0bc |
theiliad
requested review from
janchild,
vpicone and
sstrubberg
and removed request for
a team
ghost
requested review from
aledavila and
emyarod
and removed request for
a team
…te into codesandbox-dataviz
again, separate issue irrelevant to this PR. |
|
the build has not started after 45 minutes which is why github is showing the red X beside vercel, @vpicone could you restart it please? |
|
aren’t those the sandbox examples? If this is about just the layout of the page then this looks good to go other than Resource card needs to be under the anchor links ————
———— The anchor links go under the introduction paragraph then resource card like below
|
this PR's job is to add our existing sandbox examples to the carbon website. what you're mentioning would be a bug (if broken) in the actual Carbon Charts library. Also, labels were refactored by accurat, so this is either something that they missed or didn't start with as a requirement |
done @mjabbink |
|
Don’t see it reflected in the preview but assume it follows all the other pages with resource card placed under the intro and anchor links. Good to merge otherwise. |
yes, the preview is still building, probably another 10-15minutes but yes I've moved the resources card below anchor links |
|
@theiliad See it now and good to go. |
great thanks Mike |
| <a target="_blank" rel="noopener noreferrer" href={sandboxUrl}> | ||
| CodeSandbox <Launch16 /> | ||
| </a> | ||
| {showSandboxURL && ( |
There was a problem hiding this comment.
Love the re-use of the code bar. I'm confused how showSandboxURL is meant to be used though. As is, this check would mean we have to add showSandboxURL=true to all of our deployed ComponentDemos (they're currently missing the sandbox link in build preview).
What if instead, useCodesandbox returned null when no code is provided. We could use sandboxUrl && to determine whether to show the link or not.
| {showSandboxURL && ( | |
| {sandboxUrl && ( |
| ) : ( | ||
| )} | ||
|
|
||
| {!src && code && ( |
There was a problem hiding this comment.
| {!src && code && ( | |
| {shouldShowCopyButton ( |
| @@ -17,21 +17,24 @@ const StorybookLink = ({ framework, url }) => ( | |||
| </a> | |||
| ); | |||
|
|
|||
| const CodeBar = ({ src, code, links }) => { | |||
| const CodeBar = ({ src, code, links, showSandboxURL }) => { | |||
| const sandboxUrl = useCodesandbox(code); | |||
| const storybookLinks = Object.entries(links); | |||
|
|
|||
There was a problem hiding this comment.
subjective alert Do you mind moving this into a single, affirmative boolean. Just to clarify the intent a bit we can avoid chaining shortcuts. Not sure how you would have known this, it should be codified in an eslint rule.
…ite-1 into codesandbox-dataviz
all addressed, I'd probably squash & merge since there are tons of commits |
|
👍 |
* initial effort * add dynamic demos to basic charts * update charting package version, optimize demo components * fix lint error * update @carbon/charts to 0.32.3 * add image alt tags to dataviz chart types page * update yarn.lock * fix styling issues * styling fixes * undo overview-card tag height change * move resources below anchor links for basic charts page * final touch on CodeBar Co-authored-by: TJ Egan <[email protected]>
* initial effort * add dynamic demos to basic charts * update charting package version, optimize demo components * fix lint error * update @carbon/charts to 0.32.3 * add image alt tags to dataviz chart types page * update yarn.lock * fix styling issues * styling fixes * undo overview-card tag height change * move resources below anchor links for basic charts page * final touch on CodeBar Co-authored-by: TJ Egan <[email protected]>
Closes #1271