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

Fix display for search results #86040

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

GuillaumeGomez
Copy link
Member

This fixes unwanted margin and font-weight coming from .method. Before:

Screenshot from 2021-06-05 23-03-34

after:

Screenshot from 2021-06-05 23-05-02

r? @jsha

@GuillaumeGomez GuillaumeGomez added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc labels Jun 5, 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 Jun 5, 2021
@jsha
Copy link
Contributor

jsha commented Jun 6, 2021

@bors r+ rollup

Thanks for the fix! As a followup, it would be nice to change the class names for search result entries so they're not the same as class names used in the document body. For instance these could become .result-method.

@bors
Copy link
Contributor

bors commented Jun 6, 2021

📌 Commit c01bd56 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 Jun 6, 2021
@GuillaumeGomez
Copy link
Member Author

The advantage of the current situation is that we use the same colors (which is why we re-used the class in the first place).

@cynecx
Copy link
Contributor

cynecx commented Jun 6, 2021

This doesn't fix:

image

(See the top/bottom margin for type def items)

It looks like the problem is inherent to the elimination of data-level selector. We apply css we shouldn't have applied to elements and try to fix it by overwriting them by adding "more concrete opt-out rules". This has futures maintenance drawbacks (like when we add more attributes and forget to overwrite them).

I'd imagine that we should've applied these rules with more precision like we did with data-level:

.impl, .method,
.type, .associatedconstant,
.associatedtype {
flex-basis: 100%;
font-weight: 600;
margin-top: 16px;
margin-bottom: 10px;
position: relative;
}

@GuillaumeGomez
Copy link
Member Author

I never said it fixes the bold text, I said it fixes the margin on the search results.

@cynecx
Copy link
Contributor

cynecx commented Jun 6, 2021

Sorry, I didn't even notice the PR title and assumed that this PR was to fix the top/bottom margin issue. However, my points I mentioned still hold imo.

@GuillaumeGomez
Copy link
Member Author

Yes, but they are completely unrelated to this fix.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 6, 2021
…y-height, r=jsha

Fix display for search results

This fixes unwanted margin and font-weight coming from `.method`. Before:

![Screenshot from 2021-06-05 23-03-34](https://user-images.githubusercontent.com/3050060/120905486-9e46f380-c652-11eb-8008-6db6e0517ba3.png)

after:

![Screenshot from 2021-06-05 23-05-02](https://user-images.githubusercontent.com/3050060/120905489-9edf8a00-c652-11eb-817d-f676f6ab7303.png)

r? `@jsha`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#83433 (Pass --cfg=bootstrap for proc macros built by stage0)
 - rust-lang#84940 (Don't run sanity checks for `x.py setup`)
 - rust-lang#85912 (Use `Iterator::any` and `filter_map` instead of open-coding them)
 - rust-lang#85965 (Remove dead code from `LocalAnalyzer`)
 - rust-lang#86010 (Fix two ICEs in the parser)
 - rust-lang#86040 (Fix display for search results)
 - rust-lang#86058 (Remove `_`  from E0121 diagnostic suggestions)
 - rust-lang#86077 (Fix corrected example in E0759.md)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 00a704f into rust-lang:master Jun 7, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 7, 2021
@GuillaumeGomez GuillaumeGomez deleted the search-result-display-height branch June 7, 2021 08:19
@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jun 22, 2021
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) 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.

7 participants