docs(Props): make large code blocks more legible #2215
Conversation
| @@ -124,6 +124,8 @@ | |||
| font-size: 87.5%; | |||
| background-color: rgba(0, 0, 0, 0.04); | |||
| border-radius: 3px; | |||
| white-space: pre; | |||
| display: inline-block; | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
A good point. Let's go with that
|
Nice fix, thanks! I've left one comment. Also, please run |
|
It looks like all tests are passing. I'm getting a few linting warnings, but not errors, and not in anything I touched. |
Codecov Report
@@ Coverage Diff @@
## master #2215 +/- ##
=========================================
Coverage ? 99.73%
=========================================
Files ? 151
Lines ? 2624
Branches ? 0
=========================================
Hits ? 2617
Misses ? 7
Partials ? 0Continue to review full report at Codecov.
|
|
Once we get final word from @redbmk that he is happy with the |
redbmk
force-pushed
the
large-code-blocks
branch
from
cc7905b to
48a2ed1
Compare
|
Oh boy, appears the doc perf refactor just merged gives quite the conflict here. I believe the component which had the inline style has moved. Apologies, you may need to run the docs and see which component that is now. |
redbmk
force-pushed
the
large-code-blocks
branch
from
48a2ed1 to
ea316ae
Compare
|
Ah, yeah I wasn't expecting that. No worries though, I found where it needs to go. I took out all the padding rules, which do have the same effect as I merged in with the latest master and squashed it back down to a single commit. |
| @@ -50,7 +50,7 @@ export default class ComponentProps extends Component { | |||
| /> | |||
|
|
|||
| {activeName && ( | |||
| <div> | |||
| <div style={{ overflowX: 'auto' }}> | |||
There was a problem hiding this comment.
Please extract this style to variable like there. Double curly braces are performance killer as they always return the new object on each render. I'm not sute about this case, possible @levithomason can correct me
There was a problem hiding this comment.
No problem. I went ahead and moved that into a const. Even if it doesn't make a difference, it couldn't hurt.
Components with large blocks of Default code cause the table to extend beyond the page, making it difficult to read. Make the code wrap properly and allow the container to scroll. As a specific example, this can be seen in the props for Search.Result.
redbmk
force-pushed
the
large-code-blocks
branch
from
ea316ae to
8726ad3
Compare
layershifter
changed the title
|
Released in |
Components with large blocks of Default code
cause the table to extend beyond the page, making
it difficult to read. Make the code wrap properly
and allow the container to scroll.
As a specific example, this can be seen in the props
for Search.Result.