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

Improve the CSS situation in the repository #496

Merged
merged 5 commits into from
Nov 27, 2019
Merged

Improve the CSS situation in the repository #496

merged 5 commits into from
Nov 27, 2019

Conversation

pietroalbini
Copy link
Member

This PR fixes some of the CSS mess contained in this repository:

  • Removed css/style.css, as it was just an out of date copy of the generated templates/style.scss. The file was actually never served to users, as the SCSS generated on the fly takes precedence.
  • Removed the 1.40.0-nightly vendored CSS files, as they're not used by anything.
  • Removed the 1.10.0-nightly vendored CSS files, and changed the application to use the latest nightly's essential files instead of the 1.10 ones. This required a change in the Docker entrypoint to load the essential files the first time the container is started, and a few tweaks to our style.scss to workaround the changes in Rustdoc's CSS since 1.10.
  • Moved css/ to static/ in the repository, as it now only contains favicon.ico.

r? @jyn514

The file was added to the css folder by mistake, it's supposed to be
generated from templates/style.scss
They're not actually being used for the main website, and the crates
built with that nightly will load their essential files anyway.
This commit removes the vendored CSS of Rustdoc 1.10.0-nightly, changing
the site to always use the lastest Rustdoc's one. To do that, the Docker
entrypoint is changed to add the essential files the first time the
container is started, and the frontend now queries the database to see
the version of the files to include.

The version is cached by the first page loaded by users, and doesn't
change unless the application is restarted. This is fine, since the
application is restarted often enough and using a CSS older by a few
nightlies is not so big of a deal. The cache also prevents response time
regressions.
@pietroalbini
Copy link
Member Author

cc @GuillaumeGomez as well

@jyn514
Copy link
Member

jyn514 commented Nov 27, 2019

Can you also apply 60ded3b to style.scss?

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Looks good except for the one comment about the entrypoint, I think it can be avoided by rearranging the logic a little.

src/web/page.rs Show resolved Hide resolved
@pietroalbini
Copy link
Member Author

Can you also apply 60ded3b to style.scss?

Ported the configuration.

@jyn514
Copy link
Member

jyn514 commented Nov 27, 2019

Hmm, I see your changes in the CSS but the toolbar still overlaps with the item ... but that's separate from your changes here, we can fix it later.

image

@jyn514
Copy link
Member

jyn514 commented Nov 27, 2019

ah wait I'm on the wrong git branch lol one sec

@jyn514
Copy link
Member

jyn514 commented Nov 27, 2019

Ok yeah this looks great!

@jyn514 jyn514 merged commit d5b3b08 into master Nov 27, 2019
@jyn514 jyn514 deleted the css-sanity branch November 27, 2019 14:53
jyn514 added a commit to jyn514/docs.rs that referenced this pull request Nov 27, 2019
I'm not sure why it was disabled in the first place, but it started
affecting users after rust-lang#496 was
merged.
@jyn514 jyn514 mentioned this pull request Nov 27, 2019
jyn514 added a commit that referenced this pull request Nov 27, 2019
I'm not sure why it was disabled in the first place, but it started
affecting users after #496 was
merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants