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

fix(examples and docs): adding react-live… #666

Merged
merged 3 commits into from
Sep 27, 2018

Conversation

jschuler
Copy link
Collaborator

… for the docs examples

affects: patternfly4-react-lerna-root, @patternfly/react-core, @patternfly/react-docs

What:
Added react-live to react-docs for live running examples

… for the docs examples

affects: patternfly4-react-lerna-root, @patternfly/react-core, @patternfly/react-docs
@jschuler
Copy link
Collaborator Author

@patternfly-build
Copy link
Contributor

PatternFly documentation deployment: https://666-pr-patternfly-react-patternfly.surge.sh

@coveralls
Copy link

coveralls commented Sep 27, 2018

Pull Request Test Coverage Report for Build 2243

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 79.207%

Totals Coverage Status
Change from base Build 2236: 0.0%
Covered Lines: 3050
Relevant Lines: 3594

💛 - Coveralls

dgutride
dgutride previously approved these changes Sep 27, 2018
karelhala
karelhala previously approved these changes Sep 27, 2018
Copy link
Contributor

@karelhala karelhala left a comment

Choose a reason for hiding this comment

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

Awesome PR!

amarie401
amarie401 previously approved these changes Sep 27, 2018
const transformCode = code => {
try {
// LiveEditor doesn't work properly with these so need to remove
code = code.replace(/^\s*import.*$/gm, '');
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you able to do this on gatsby node? It might speed up client-side rendering. Not a big deal if it is not possible though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion, I will try it out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had trouble getting the raw file contents in gatsby-node. Going to stick with this for now.

code = code.replace(/extends Component/gm, 'extends React.Component');
code = code.replace(/^\s*export.*$/gm, '');
code = code.replace(/^\s*static.*$/gm, '');
const transformedCode = transform(code, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Internally it looks like React-Live uses (buble)[https://buble.surge.sh/guide/#jsx] which supports jsx. I did not try this without babel so it might actually be needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I had more issues with buble so I switched to babel

@@ -32,12 +32,17 @@ const Layout = ({ children, data }) => {

return (
<React.Fragment>
<Helmet
{/* <Helmet
Copy link
Contributor

Choose a reason for hiding this comment

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

This can either be removed or readded. Might be nice to include the meta tags though. You should be able to keep the links that you added as children.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added them back in

@@ -2,8 +2,8 @@ import React from 'react';
import { Badge } from '@patternfly/react-core';
import getContainerProps from './common/getContainerProps';

class UnreadBadge extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these changes were already made with pr #662 that was merged yesterday.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah ok good, I will merge with master and repush

@jschuler jschuler dismissed stale reviews from amarie401, karelhala, and dgutride via 1fe996e September 27, 2018 16:18
@jschuler
Copy link
Collaborator Author

Added env variables so it can also be disabled if needed. Btw #666 😈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants