-
Notifications
You must be signed in to change notification settings - Fork 840
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
Adds ability to copy EuiCodeBlock and adds snippet prop to docs #1556
Conversation
cc @sqren who requested a somewhat similar feature many months ago and may have interest. |
Yay! This looks so great. Very improvement, much like! |
Feedback
|
2a14f17
to
efb1173
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am quite excited for this new tab!
Co-Authored-By: ryankeairns <[email protected]>
@cchaos @chandlerprall PR has been updated and is ready for a follow-up review. Thanks! |
802db77
to
08083e2
Compare
@ryankeairns Should we drop this docs example into a snippet now? Think the callout still makes sense though. |
@snide I have moved the fixed progress example to the Snippet tab. It utilizes two examples in this case, separating the |
@ryankeairns, Your last comment makes me think that maybe we will want to allow an array of snippets? Mainly just to separate out these different types so they each have their own copy button. |
@cchaos That would be a nice enhancement! If you don't mind, I'd like to move that to a separate/follow-up item in order to get this initial functionality out. I'm needing to dig in on some unrelated work ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one question about a background color
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one last nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautiful :) Code changes LGTM, I tested locally and everything works as expected
Summary
This PR has two related objectives:
EuiCodeBlock
via anisCopyable
prop.snippet
prop toGuideSection
that optionally displays a new Snippet tab. This essentially allows an EUI docs user to copy code without having to wade through all the demo code in the Javascript tab. It won't be appropriate for all components, but likely for the majority. See it in action on the Code and Progress pages.Add copy button
Works with fullscreen too
Copyable snippet
^ Something to consider is that what we provide in the snippet may be proliferated. In other words, we should likely put the preferred settings in there. The catch to that is we'd like to demonstrate some available props which, in the preferred state, commonly use the defaults. The moral of the story is to be thoughtful with what we put into the Snippet tab.
Snippets in EUI docs
By using a prop on
GuideSection
, we can add a snippet easily in the*_example.js
file and produce consistent output (i.e. the same size, padding, copy button, etc., on theEuiCodeBlock
):Checklist
Jest tests were updated or added to match the most common scenariosThis required updates to Framer X components