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

feat(docs): use gatsby-theme-patternfly-org #3146

Merged
merged 36 commits into from Oct 18, 2019
Merged

Conversation

@redallen
Copy link
Contributor

redallen commented Oct 15, 2019

Preview link: https://patternfly-react-pr-gatsby-theme-patternfly-org.surge.sh/patternfly-4/

What: Refactor docs to work with gatsby-theme-patternfly-org. Closes #2775, closes #2738, towards patternfly/patternfly-org#1208

New features:

Additional issues:

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 15, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@3a965e4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3146   +/-   ##
=========================================
  Coverage          ?   68.98%           
=========================================
  Files             ?      858           
  Lines             ?    23627           
  Branches          ?     1893           
=========================================
  Hits              ?    16300           
  Misses            ?     6366           
  Partials          ?      961
Flag Coverage Δ
#misc 95.45% <ø> (?)
#patternfly3 69.3% <ø> (?)
#patternfly4 67.95% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a965e4...467a161. Read the comment docs.

redallen and others added 3 commits Oct 16, 2019
…atternfly-react into gatsby-theme-patternfly-org
Copy link
Member

dlabrecq left a comment

I'm going to stop reviewing here for now. The https://patternfly-react-gatsby-theme.surge.sh link doesn't appear to be in sync with the latest changes?

I understand the refactoring for the site, but feel the reformatting of titles, docs, etc. may be out of scope?

I don't see any API docs for charts and dark mode doesn't seem to be apply supported.

import React from 'react';
import { Chart, ChartBar } from '@patternfly/react-charts';
<div>
<div className="bar-chart-legend-right">
RightLegend = (

This comment has been minimized.

Copy link
@dlabrecq

dlabrecq Oct 16, 2019

Member

We have both RightLegend and RightAlignedLegend, which sound too similar. Could this be BarChartSimple. The prefix would help identify the example, especially while cutting/pasting examples

This comment has been minimized.

Copy link
@redallen

redallen Oct 16, 2019

Author Contributor

We could do this, but our current variable names for classes don't do this. Example: class MultiColorChart extends React.Component { in ChartThreshold.md does not include a Threshold prefix. I'm just going to keep shorter variable names for now that match the titles.

Edit: Fixing these specific examples because they have the exact same title and that's bad.

This comment has been minimized.

Copy link
@dlabrecq

dlabrecq Oct 16, 2019

Member

I'm not following the logic as to what these names must be? We can make the class names match whatever we choose -- that shouldn't be a deciding factor.

If I were to clean up afterward, can I not use BarChartSimple, BarChartMultiColor, BarCharRightLegend, etc.?

This comment has been minimized.

Copy link
@redallen

redallen Oct 17, 2019

Author Contributor

These can be anything. The only thing they matter for is what people copy/paste.

@dlabrecq

This comment has been minimized.

Copy link
Member

dlabrecq commented Oct 17, 2019

Discovered an issue with codesandbox. Seeing a "DependencyNotFoundError".

Could not find dependency: '@patternfly/react-charts' relative to '/index.js'

And the "Toggle dark theme" mode does not work with charts -- we don't support a dark theme, yet

redallen added 6 commits Oct 17, 2019
@redallen redallen removed the Do Not Merge label Oct 17, 2019
@redallen redallen requested review from dgutride and dlabrecq Oct 17, 2019
@evwilkin

This comment has been minimized.

Copy link
Contributor

evwilkin commented Oct 17, 2019

Discovered an issue with codesandbox. Seeing a "DependencyNotFoundError".

Could not find dependency: '@patternfly/react-charts' relative to '/index.js'

@redallen I'm seeing a similar error when using the codesandbox link for the Toolbar layout:
Could not fetch version for @patternfly/react-toplogy@latest

@redallen

This comment has been minimized.

Copy link
Contributor Author

redallen commented Oct 17, 2019

@evwilkin That's on the followup checklist!

@tlabaj
tlabaj approved these changes Oct 18, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@dlabrecq dlabrecq merged commit 93c843b into master Oct 18, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Oct 18, 2019

Your changes have been released in:

  • @patternfly/react-charts@5.1.0
  • @patternfly/react-core@3.117.0
  • @patternfly/react-docs@4.15.0
  • gatsby-transformer-react-docgen-typescript@0.2.0
  • @patternfly/react-inline-edit-extension@2.12.0
  • demo-app-ts@3.7.14
  • @patternfly/react-styled-system@3.7.0
  • @patternfly/react-styles@3.6.0
  • @patternfly/react-table@2.24.0
  • @patternfly/react-tokens@2.7.0
  • @patternfly/react-topology@2.10.0
  • @patternfly/react-virtualized-extension@1.3.0

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.