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.

Sign up for GitHub

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

docs(Props): make large code blocks more legible #2215

Merged
merged 2 commits into from
Oct 22, 2017
Merged

docs(Props): make large code blocks more legible #2215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8726ad3
docs: make large code blocks more legible
redbmk Oct 19, 2017
604ae45
style(docs): change variable name
layershifter Oct 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/app/Components/ComponentDoc/ComponentProps/ComponentProps.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import ComponentPropsComponents from './ComponentPropsComponents'
import ComponentPropsDescription from './ComponentPropsDescription'
import ComponentPropsHeader from './ComponentPropsHeader'

const propsContainerStyle = { overflowX: 'auto' }

export default class ComponentProps extends Component {
static propTypes = {
componentGroup: PropTypes.arrayOf(PropTypes.object),
Expand Down Expand Up @@ -50,7 +52,7 @@ export default class ComponentProps extends Component {
/>

{activeName && (
<div>
<div style={propsContainerStyle}>
<ComponentPropsDescription description={description} />
<ComponentTable name={activeName} props={props} />
</div>
Expand Down
6 changes: 2 additions & 4 deletions docs/app/index.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,11 @@
}

code:not(.hljs) {
padding: 0;
padding-top: 0.1em;
padding-bottom: 0.2em;
margin: 0;
font-size: 87.5%;
background-color: rgba(0, 0, 0, 0.04);
border-radius: 3px;
white-space: pre;
display: inline-block;
Copy link
Member

Choose a reason for hiding this comment

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

With proper whitespace, now the padding-top and padding-bottom need to be adjusted to match. The block element now has too much bottom padding.

Let's go with padding: 0.1em 0 instead of the 3 rules currently in place (see line 120-123).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. After looking at it some more though, it looks like with padding: 0.1em 0, display: inline-block pushes the text up. Try setting both those rules in e.g. the Icon docs, then toggle the display setting on and off in dev tools - you'll see the text bounce up and down. If we use padding: 0 though, the code padding will still look larger but without pushing the text around.

It's your call though - just let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and added the padding: 0.1em 0 as a separate commit. Let me know if you want me to change it or squash them.

Copy link
Member

Choose a reason for hiding this comment

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

I see what you're saying. I think padding: 0; looks great. In fact, removing all padding rules may be the same thing in this case. Try it with no padding rules and see what you think.

I'll let you make the call and we'll ship it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A good point. Let's go with that

}

code:not(.hljs)::before {
Expand Down