-
Notifications
You must be signed in to change notification settings - Fork 272
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
Gatsby v5 : code snippet bugs #1439
Comments
Hi @muditjuneja thanks for all your help with this so far! Not sure if you have time to take a look at any of the above issues I noticed. Its so close |
Hey @alisonjoseph I will look into these soon. |
can you clarify the this - |
Hi @eng618 the styles are not being applied properly on the copy button due to the conflicts in styling. I guess, All those webpack warnings when building and starting the dev loop are playing a role here. Do you know what are all these conflicts? |
No I’m not familiar with that, and the styling isn’t really my strong area. @alisonjoseph is there anyone who can help take a look at this? |
I can take a look and get a fix in for the icon color. With regards to the new |
Yeah that makes sense. Actually there is already a logic that if there are 9 lines of code, it would automatically show the button and hide the code that we changed in the next branch. I can change the defaultValue to |
@muditjuneja oh ok, yes I think having it default to true would be great. thank you! |
@muditjuneja looking at this locally and the hideCode prop doesn't appear to be doing anything at all, curious why we need this at all? What would be the use-case for someone to not want the show more button for longer code blocks? |
If there is a code block having more than 9 lines of code then this would come into play. |
@muditjuneja got it, I just opened a PR with an update to the docs page, I don't believe it is working. |
This still seems a little clunky and not intuitive... Just thinking this out... what if instead of this boolean logic, we default to always showing the Show More button, but if we add a prop Similar to Caption: https://gatsby.carbondesignsystem.com/components/Caption with the @muditjuneja thoughts? |
fullWidth is also a bool but I think showAll makes more sense as the prop name. Here is the PR for this change - #1445 |
It is a supported language though: https://prismjs.com/#supported-languages |
Actually, looks like we would have to add that manually... they only include these base languages by default: https://github.com/FormidableLabs/prism-react-renderer/blob/c914fdea48625ba59c8022174bb3df1ed802ce4d/packages/generate-prism-languages/index.ts#L9-L23
|
Moving over code snippet specific bugs from #1425 in addition to some new ones.
The text was updated successfully, but these errors were encountered: