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: remove double imports of css #2260

Merged
merged 1 commit into from
May 13, 2024

Conversation

KFearsoff
Copy link
Contributor

Currently, the CSS stylesheets that are already imported from the parent HTML are also imported from the CSS stylesheets. As far as I can tell, it doesn't cause any major bugs, but it's just messy and causes minor issues (for example, Firefox doesn't fire extra requests to fetch CSS twice, but it loads it twice which makes navigating developer menu harder).

From my not-very-extensive testing, it should also allows us to remove this script:

<!-- Set the theme before any content is loaded, prevents flash -->
<script>
var theme;
try { theme = localStorage.getItem('mdbook-theme'); } catch(e) { }
if (theme === null || theme === undefined) { theme = default_theme; }
var html = document.querySelector('html');
html.classList.remove('{{ default_theme }}')
html.classList.add(theme);
var body = document.querySelector('body');
body.classList.remove('no-js')
body.classList.add('js');
</script>

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Dec 6, 2023
Copy link
Contributor

@jhult jhult left a comment

Choose a reason for hiding this comment

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

LGTM

@ehuss or @leonzchang, can we merge this?

@jhult
Copy link
Contributor

jhult commented May 13, 2024

@ehuss, any thoughts on this?

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue May 13, 2024
@ehuss
Copy link
Contributor

ehuss commented May 13, 2024

PRs like these can take a while to review. In the future, it could help to provide more information. For example:

  • What exact issue is this trying to solve? What "developer menu" is this referring to? What does a screenshot look like?
  • What is an @import? How does it differ from <link>? Why would one choose one over the other? Can you cite MDN articles or others that would help explain the choices here?
  • Why was @import added in the first place?

Merged via the queue into rust-lang:master with commit 09576d7 May 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants