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.

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

Merged
merged 11 commits into from
Feb 14, 2019

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Feb 12, 2019

Summary

This PR has two related objectives:

  1. Add an optional 'copy' icon button to EuiCodeBlock via an isCopyable prop.
  2. Add a snippet prop to GuideSection 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

screenshot 2019-02-12 11 59 43

Works with fullscreen too

screenshot 2019-02-12 12 00 39

Copyable snippet

screenshot 2019-02-13 09 55 26

^ 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 the EuiCodeBlock):

const codeSource = require('!!raw-loader!./code');
const codeHtml = renderToHtml(Code);
const codeSnippet = '<EuiCode>Text to be formatted</EuiCode>';
...
    text: (
      <p>
        <EuiCode>Code</EuiCode> is for making inline code snippets that can work
        within or next to bodies of text.
      </p>
    ),
    snippet: codeSnippet,
...

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios
  • This required updates to Framer X components

@ryankeairns ryankeairns added documentation Issues or PRs that only affect documentation - will not need changelog entries work in progress labels Feb 12, 2019
@snide
Copy link
Contributor

snide commented Feb 12, 2019

cc @sqren who requested a somewhat similar feature many months ago and may have interest.

@ryankeairns
Copy link
Contributor Author

Soren also had a recent request for something like this in relation to the progress bar positioning over the header. While this specific example uses a slightly different implementation (+ an EuiCallout), it does produce a very similar solution:

screenshot 2019-02-12 12 17 50

@sorenlouv
Copy link
Member

Yay! This looks so great. Very improvement, much like!

@ryankeairns
Copy link
Contributor Author

Feedback

  • Place in separate tab titled Snippet
  • Think about rewording JavasScript tab (Demo code?)
  • Consider which props to show/not show in that new tab

@ryankeairns ryankeairns changed the title [WIP] Adds ability to copy EuiCodeBlock and adds snippet prop to docs Adds ability to copy EuiCodeBlock and adds snippet prop to docs Feb 13, 2019
src-docs/src/components/guide_section/guide_section.js Outdated Show resolved Hide resolved
src/components/code/_code_block.js Outdated Show resolved Hide resolved
src/components/code/_code_block.js Show resolved Hide resolved
Copy link
Contributor

@cchaos cchaos left a 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!

src/components/code/_code_block.js Outdated Show resolved Hide resolved
src/components/code/_code_block.scss Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: ryankeairns <[email protected]>
@ryankeairns
Copy link
Contributor Author

@cchaos @chandlerprall PR has been updated and is ready for a follow-up review. Thanks!

@snide
Copy link
Contributor

snide commented Feb 14, 2019

@ryankeairns Should we drop this docs example into a snippet now? Think the callout still makes sense though.

image

@ryankeairns
Copy link
Contributor Author

@snide I have moved the fixed progress example to the Snippet tab.

It utilizes two examples in this case, separating the absolute and fixed options to emphasize when the portal may be necessary. Each example has a short comment to help clarify, similar to how Zurb Foundation documents things:

screenshot 2019-02-14 08 12 16

@cchaos
Copy link
Contributor

cchaos commented Feb 14, 2019

@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.

@ryankeairns
Copy link
Contributor Author

@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 ;)

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Progress Demo HTML is empty. I don't see anything in your changes that's likely to have caused it, though
screen shot 2019-02-14 at 10 10 19 am

Copy link
Contributor

@cchaos cchaos left a 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

src/components/code/_code_block.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

one last nit

src/components/code/_code_block.js Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerprall chandlerprall left a 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

@ryankeairns ryankeairns merged commit 8c50c5d into elastic:master Feb 14, 2019
@ryankeairns
Copy link
Contributor Author

tenor-97962450

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants