Fixed button wrapping behavior on mobile #921
Conversation
previous+next buttons with long text now stack properly on smaller screens
|
Deploy preview for docusaurus-preview ready! Built with commit 320fde7 |
Function/component names are truncated, while regular titles are wrapped
| @@ -1545,8 +1545,28 @@ input::placeholder { | |||
| } | |||
|
|
|||
| @media only screen and (max-width: 735px) { | |||
| .docs-prevnext { | |||
There was a problem hiding this comment.
Deleting this means the class here is useless
https://github.com/notjaril/Docusaurus/blob/f9151ca17ac424d0102d65faedef78db15ef4cbd/lib/core/DocsLayout.js#L90
<div className="docs-prevnext">There was a problem hiding this comment.
The docs-prevnext class is still styled with:
/* Start of Docs Navigation */
.docs-prevnext {
margin: 20px 0;
}The intent with removing the doc-prevnext styling within the media query was so the buttons' top and bottom margins were displayed properly, where it wasn't being displayed properly with a fixed doc-prevnext height.
Does this make sense? Let me know if I missed something.
There was a problem hiding this comment.
This is correct. I missed that 👍
lib/core/DocsLayout.js
Outdated
| className={ | ||
| nextTitle.match(/[a-z][A-Z]/) | ||
| ? 'function-name-prevnext' | ||
| : 'text-next' |
There was a problem hiding this comment.
I couldn't find any text-next css class on the new main.css.
There was a problem hiding this comment.
You're right, thanks for checking - it was leftover styling from something I was trying before. Will remove it, and leave the span classless in the case that the button text is (likely) not a function/component name.
There was a problem hiding this comment.
Seems that is the case. Just checking .. It's easy to miss css selector like this 😄
There was a problem hiding this comment.
Looks good to me. However, one thing that I notice is for text that are quite short, I prefer the before looks.
Before vs after
Anyway, I'll let @yangshun or @JoelMarcey to approve this as well before merging
|
I like the 'before' look as well for shorter text. Ideally, it would keep both inline until such time that one of the buttons wrap, at which point if floats both buttons left. The problem is I don't think there's a way in CSS to toggle the styling based on whether or not the elements are wrapped. The 'after' styling just toggles automatically at screen width < 735px, floating both buttons left. If you feel strongly about keeping both buttons inline, we could do Option 1 instead of Option 2. Then both buttons would have equal widths, and any lengthy text would wrap. It's possible I'm missing something - if there's a CSS trick out there that could do that, let me know. |
|
@notjaril |
|
Fixed the conflicts - let me know if this does the trick. |
lib/core/DocsLayout.js
Outdated
| @@ -46,6 +46,23 @@ class DocsLayout extends React.Component { | |||
| const title = | |||
| idx(i18n, ['localized-strings', 'docs', id, 'title']) || defaultTitle; | |||
| const hasOnPageNav = this.props.config.onPageNav === 'separate'; | |||
|
|
|||
| const previousTitle = i18n | |||
| ? translation[this.props.metadata.language]['localized-strings'][ | |||
There was a problem hiding this comment.
You should do it with idx (safe getter) now. See the upstream master
passed through prettier and all tests pass
|
That was my bad - it's now fixed and is using idx. |
There was a problem hiding this comment.
I kinda like the buttons having the same width to my eye, but I don't feel strongly about it right now.
@yangshun what do you think?
Motivation
Fixes #918
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
It should look like this:
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)