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.

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

Move tooltips messages out of html #79742

Merged

Conversation

GuillaumeGomez
Copy link
Member

First thing first: nothing in the output has changed. You still have the "i" on the left of code blocks examples when they have ignore, compile_fail, should_panic and edition. The behavior also remains the same: when you hover the "i", you have the corresponding message showing up.

So now, why this PR then? I realized recently that we were actually generating those messages into the HTML every time whereas all messages are the same (except for the edition ones, I'll come back to it later). So instead of generating more content, I simply moved it inside the CSS thanks to pseudo elements (::before and ::after). The message is now inside ::after and we use the ::before to have the small triangle on the left of the message. So now, we have less HTML generated which is seems pretty nice.

So now, back to the edition change: the message is globally the same, but the "edition" itself can be different (2015 or 2018 currently, I expect 2021 to arrive not too far in the future). So the only difference for it is that I added a new attribute on the tooltip called edition which contains this information. Then, the ::after uses it inside its content (you can get the content of an element's attribute by using attr and concat different strings by simply having them after the other).

Don't hesitate if a part of my explanations isn't clear.

r? @jyn514

@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 Dec 5, 2020
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A change occurred in the Ayu theme.

cc @Cldfire

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2020
@GuillaumeGomez GuillaumeGomez force-pushed the move-tooltips-messages-out-of-html branch from d484660 to 9b62936 Compare December 5, 2020 19:15
@Nemo157
Copy link
Member

Nemo157 commented Dec 23, 2020

I haven't checked that it works properly, but the approach seems sane to me. One thing I might do is change edition to data-edition, I'm not sure what the standard says about non-standard attributes outside the data-* namespace.

@GuillaumeGomez
Copy link
Member Author

No idea either. Renaming to data-edition if it's more "standard" then!

@GuillaumeGomez
Copy link
Member Author

From discord (for explanations about @Nemo157's suggestion):

In the next example, there is a non-conforming attribute value ("carpet") and a non-conforming attribute ("texture"), which is not permitted by this specification:
<label>Carpet: <input type="carpet" name="c" texture="deep pile"></label>

Here would be an alternative and correct way to mark this up:
<label>Carpet: <input type="text" class="carpet" name="c" data-texture="deep pile"></label>

-- https://html.spec.whatwg.org/#semantics-2

@GuillaumeGomez
Copy link
Member Author

Ok let's go!

@bors: r=Nemo157

@bors
Copy link
Contributor

bors commented Dec 23, 2020

📌 Commit 152d4e7 has been approved by Nemo157

@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 Dec 23, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 23, 2020
…es-out-of-html, r=Nemo157

Move tooltips messages out of html

First thing first: nothing in the output has changed. You still have the "i" on the left of code blocks examples when they have `ignore`, `compile_fail`, `should_panic` and `edition`. The behavior also remains the same: when you hover the "i", you have the corresponding message showing up.

So now, why this PR then? I realized recently that we were actually generating those messages into the HTML every time whereas all messages are the same (except for the edition ones, I'll come back to it later). So instead of generating more content, I simply moved it inside the CSS thanks to pseudo elements (`::before` and `::after`). The message is now inside `::after` and we use the `::before` to have the small triangle on the left of the message. So now, we have less HTML generated which is seems pretty nice.

So now, back to the `edition` change: the message is globally the same, but the "edition" itself can be different (2015 or 2018 currently, I expect 2021 to arrive not too far in the future). So the only difference for it is that I added a new attribute on the tooltip called `edition` which contains this information. Then, the `::after` uses it inside its `content` (you can get the content of an element's attribute by using `attr` and concat different strings by simply having them after the other).

Don't hesitate if a part of my explanations isn't clear.

r? `@jyn514`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 24, 2020
…es-out-of-html, r=Nemo157

Move tooltips messages out of html

First thing first: nothing in the output has changed. You still have the "i" on the left of code blocks examples when they have `ignore`, `compile_fail`, `should_panic` and `edition`. The behavior also remains the same: when you hover the "i", you have the corresponding message showing up.

So now, why this PR then? I realized recently that we were actually generating those messages into the HTML every time whereas all messages are the same (except for the edition ones, I'll come back to it later). So instead of generating more content, I simply moved it inside the CSS thanks to pseudo elements (`::before` and `::after`). The message is now inside `::after` and we use the `::before` to have the small triangle on the left of the message. So now, we have less HTML generated which is seems pretty nice.

So now, back to the `edition` change: the message is globally the same, but the "edition" itself can be different (2015 or 2018 currently, I expect 2021 to arrive not too far in the future). So the only difference for it is that I added a new attribute on the tooltip called `edition` which contains this information. Then, the `::after` uses it inside its `content` (you can get the content of an element's attribute by using `attr` and concat different strings by simply having them after the other).

Don't hesitate if a part of my explanations isn't clear.

r? ``@jyn514``
@bors
Copy link
Contributor

bors commented Dec 24, 2020

⌛ Testing commit 152d4e7 with merge b251612...

@bors
Copy link
Contributor

bors commented Dec 24, 2020

☀️ Test successful - checks-actions
Approved by: Nemo157
Pushing b251612 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 24, 2020
@bors bors merged commit b251612 into rust-lang:master Dec 24, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 24, 2020
@GuillaumeGomez GuillaumeGomez deleted the move-tooltips-messages-out-of-html branch December 24, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants