FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

chore(react-docs): updated react-docs to use Red Hat font by tlabaj · Pull Request #2563 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

chore(react-docs): updated react-docs to use Red Hat font - #2563

Merged
redallen merged 5 commits into
patternfly:masterfrom
tlabaj:font_updates
Jul 22, 2019
ConversationCommits (5)ChecksFiles changed
Merged

chore(react-docs): updated react-docs to use Red Hat font#2563
redallen merged 5 commits into
patternfly:masterfrom
tlabaj:font_updates

Conversation

tlabaj commented Jul 22, 2019

Copy link
Copy Markdown
Contributor

#1936

What:

Additional issues:

tlabaj requested review from dgutride, dlabaj and mcoker July 22, 2019 18:36

patternfly-build commented Jul 22, 2019

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-2563.surge.sh

redallen approved these changes Jul 22, 2019
View reviewed changes

redallen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I got to relearn how exactly how Gatsby renders html.js recently. Thank goodness we don't have to worry about how it statically renders it :)

Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
/>
{props.headComponents}
</head>
<body {...props.bodyAttributes} className="pf-m-redhat-font">

redallen Jul 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's possible to globally include SASS that adds that styling to body, but I like this better to give us more control of the template in the future.

dgutride Jul 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think for now we don't really need to add a template here, this docs site isn't going to last forever anyway once we can move to just using pf.org. If we use react-helmet, it alleviates the need for this entire file. @redallen

mcoker reviewed Jul 22, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-core/README.md Outdated
All css related to each component is provided within it. There is no component level CSS to import.

# Applying Red Hat Fonts (Optional)
If you would like to add Red Hat font instead of Overpass, simply add the class `.pf-m-redhat-font` to an element that wraps your application (ideally `<html>` or `<body>`) to adopt the CSS changes that introduce the Red Hat font into PatternFly.

mcoker Jul 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Small change, wdyt?

Suggested change
If you would like to add Red Hat font instead of Overpass, simply add the class `.pf-m-redhat-font` to an element that wraps your application (ideally `<html>` or `<body>`) to adopt the CSS changes that introduce the Red Hat font into PatternFly.
If you would like to use Red Hat fonts instead of Overpass, simply add the class `.pf-m-redhat-font` to an element that wraps your application (ideally `<html>` or `<body>`) to adopt the CSS changes that introduce the Red Hats font into PatternFly.

mcoker Jul 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

oops. I made a typo

Suggested change
If you would like to add Red Hat font instead of Overpass, simply add the class `.pf-m-redhat-font` to an element that wraps your application (ideally `<html>` or `<body>`) to adopt the CSS changes that introduce the Red Hat font into PatternFly.
If you would like to use Red Hat fonts instead of Overpass, simply add the class `.pf-m-redhat-font` to an element that wraps your application (ideally `<html>` or `<body>`) to adopt the CSS changes that introduce the Red Hat fonts into PatternFly.

redallen suggested changes Jul 22, 2019
View reviewed changes

redallen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Just some linter nitpicking. You can run yarn lint:js to verify and/or yarn lint:fix to fix the auto-fixable ones.

Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
dgutride reviewed Jul 22, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-docs/src/html.js Outdated
/>
{props.headComponents}
</head>
<body {...props.bodyAttributes} className="pf-m-redhat-font">

dgutride Jul 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We are using react-helmet in other places to alleviate the need for this - you can add this instead (only the body element needs to be added, other code added for reference so you can find it):

in react-docs/src/templates/sidebarLayout.js

<Helmet>
  <html lang="en-US" />
  <meta charSet="utf-8" />
  <meta name="description" content="PatternFly React Documentation" />
  <meta name="keywords" content="React, PatternFly, Red Hat" />
  <link rel="icon" type="image/x-icon" href="/favicon.ico" />
  <body className='pf-m-redhat-font' />
</Helmet>

tlabaj Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I followed these instructions from the Gatsby documentations. Are you saying that I do not need to copy the html.js file?
https://www.gatsbyjs.org/docs/custom-html/

dgutride Jul 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No - the html.js file isn't needed for just adding a class on the body tag, react-helmet supports that with a single line change to sidebarLayout

patternfly-build commented Jul 22, 2019

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-2563.surge.sh

patternfly-build commented Jul 22, 2019

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-2563.surge.sh

redallen approved these changes Jul 22, 2019
View reviewed changes

redallen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🎯 Very precise. 🎯

mcoker approved these changes Jul 22, 2019
View reviewed changes

mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm!

redallen merged commit 968fd2f into patternfly:master Jul 22, 2019

patternfly-build commented Jul 22, 2019

Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-core@3.75.1
  • @patternfly/react-docs@4.9.1
  • @patternfly/react-inline-edit-extension@2.9.48
  • demo-app-ts@2.12.10
  • @patternfly/react-table@2.14.22
  • @patternfly/react-topology@2.6.19
  • @patternfly/react-virtualized-extension@1.1.81

Thanks for your contribution! 🎉

tlabaj mentioned this pull request Jul 22, 2019
Closed
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcoker mcoker approved these changes
dlabaj Awaiting requested review from dlabaj
+2 more reviewers
dgutride dgutride left review comments
redallen redallen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

dlabaj

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL