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

Jump to bottom

Fix docblock code display on mobile #89632

Merged
merged 2 commits into from
Oct 11, 2021
Merged

Fix docblock code display on mobile #89632

merged 2 commits into from
Oct 11, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Oct 7, 2021 •
edited
Loading

Fixes #89618.

Before:

Screenshot from 2021-10-07 12-01-37

After:

Screenshot from 2021-10-07 20-17-21

r? @jsha

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Oct 7, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 7, 2021
@jsha
Copy link
Contributor

jsha commented Oct 7, 2021

Thanks for writing! See my comment: #89618 (comment). I think we should use overflow-wrap: anywhere.

@GuillaumeGomez
Copy link
Member Author

Updated! (I also updated the image.)

@jsha
Copy link
Contributor

jsha commented Oct 7, 2021

@bors r=me

@GuillaumeGomez
Copy link
Member Author

@bors: r=jsha

@bors
Copy link
Contributor

bors commented Oct 7, 2021

📌 Commit a518d1d778e1a12344862920ba236b8d228daef4 has been approved by jsha

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 7, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member Author

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 7, 2021
@GuillaumeGomez
Copy link
Member Author

@dns2utf8 Just like I told you. ^ :)

@GuillaumeGomez
Copy link
Member Author

Thanks to @dns2utf8's help, we were able to find that a long <code> in a docblock-short element was actually impacting the "table". So not linked to this PR. I simply updated the element on which I was putting the long <code> to be a module instead of a function to not break the other test.

@dns2utf8
Copy link
Contributor

An alternative would be to insert a very, very long text in the <code>
image

@GuillaumeGomez
Copy link
Member Author

I prefer to not change the layout too much considering that it's not what's being tested here. ;)

@dns2utf8
Copy link
Contributor

Good point 👍

dns2utf8
dns2utf8 reviewed Oct 10, 2021
View reviewed changes
src/librustdoc/html/static/css/rustdoc.css
@@ -1969,4 +1969,8 @@ details.undocumented[open] > summary::before {
.docblock {
margin-left: 12px;
}

.docblock code {
overflow-wrap: anywhere;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add that rule for all the layouts? If someone tries to break the record for longest symbol name we could allow breaking on desktop too

Copy link
Member Author

Choose a reason for hiding this comment

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

There is already a rule for desktop, a bit less restrictive but that prevents it from being an issue there.

@GuillaumeGomez
Copy link
Member Author

@bors: r=jsha

@bors
Copy link
Contributor

bors commented Oct 10, 2021

📌 Commit d4f3cf0 has been approved by jsha

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 10, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 10, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 11, 2021
@bors
Auto merge of rust-lang#89752 - matthiaskrgr:rollup-v4fgmwg, r=matthi…
1ddd4e6 
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#89579 (Add regression test for issue 80108)
 - rust-lang#89632 (Fix docblock code display on mobile)
 - rust-lang#89691 (Move `DebuggerCommands` and `check_debugger_output` to a separate module)
 - rust-lang#89707 (Apply clippy suggestions for std)
 - rust-lang#89722 (Fix spelling: Cannonical -> Canonical)
 - rust-lang#89736 (Remove unused CSS rule)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3fa897d into rust-lang:master Oct 11, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 11, 2021
@GuillaumeGomez GuillaumeGomez deleted the fix-docblock-code branch October 11, 2021 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dns2utf8 dns2utf8 dns2utf8 left review comments

@jsha jsha jsha approved these changes

Assignees

@jsha jsha

Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.57.0
Development

Successfully merging this pull request may close these issues.

<code> in docblocks should use "overflow-wrap" CSS rule
7 participants
@GuillaumeGomez @rust-highfive @jsha @bors @rust-log-analyzer @dns2utf8 @rustbot