Skip to content

rustdoc: clean up storage.js #109542

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

Merged
merged 2 commits into from
Mar 25, 2023
Merged

rustdoc: clean up storage.js #109542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba3d605
rustdoc: clean up `storage.js`
notriddle Mar 23, 2023
95ef91c
rustdoc: remove old `content` hack for theme switching
notriddle Mar 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions src/librustdoc/html/static/css/rustdoc.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,6 @@
box-sizing: border-box;
}

/* This part handles the "default" theme being used depending on the system one. */
html {
content: "";
Copy link
Member

Choose a reason for hiding this comment

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

Do you remember what it was used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used here:

https://github.com/rust-lang/rust/pull/109542/files#diff-0c4c5f4709d4f85219afe9ed48a00370c8de11eed5fb3ebee6c683ebb391a72bL217

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see. Why was it removed then? Or why did we add it in the first place? (Just so I can understand this change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added to support Safari back in 2020.

Copy link
Member

Choose a reason for hiding this comment

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

It's not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current version of Safari supports window.matchMedia.

Copy link
Member

Choose a reason for hiding this comment

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

Since 2012 apparently so we can remove it. Thanks for the explanations!

}
@media (prefers-color-scheme: light) {
html {
content: "light";
}
}
@media (prefers-color-scheme: dark) {
html {
content: "dark";
}
}

/* General structure and fonts */

body {
Expand Down Expand Up @@ -1538,7 +1523,7 @@ However, it's not needed with smaller screen width because the doc/code block is
/*
WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY
If you update this line, then you also need to update the line with the same warning
in storage.js
in main.js
*/
@media (max-width: 700px) {
/* When linking to an item with an `id` (for instance, by clicking a link in the sidebar,
Expand Down
5 changes: 5 additions & 0 deletions src/librustdoc/html/static/js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

"use strict";

// WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY
// If you update this line, then you also need to update the media query with the same
// warning in rustdoc.css
window.RUSTDOC_MOBILE_BREAKPOINT = 700;

// Given a basename (e.g. "storage") and an extension (e.g. ".js"), return a URL
// for a resource under the root-path, with the resource-suffix.
function resourcePath(basename, extension) {
Expand Down
74 changes: 13 additions & 61 deletions src/librustdoc/html/static/js/storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,22 @@
const darkThemes = ["dark", "ayu"];
window.currentTheme = document.getElementById("themeStyle");

// WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY
// If you update this line, then you also need to update the media query with the same
// warning in rustdoc.css
window.RUSTDOC_MOBILE_BREAKPOINT = 700;

const settingsDataset = (function() {
const settingsElement = document.getElementById("default-settings");
if (settingsElement === null) {
return null;
}
const dataset = settingsElement.dataset;
if (dataset === undefined) {
return null;
}
return dataset;
return settingsElement && settingsElement.dataset ? settingsElement.dataset : null;
})();

function getSettingValue(settingName) {
const current = getCurrentValue(settingName);
if (current !== null) {
return current;
}
if (settingsDataset !== null) {
if (current === null && settingsDataset !== null) {
// See the comment for `default_settings.into_iter()` etc. in
// `Options::from_matches` in `librustdoc/config.rs`.
const def = settingsDataset[settingName.replace(/-/g,"_")];
if (def !== undefined) {
return def;
}
}
return null;
return current;
}

const localStoredTheme = getSettingValue("theme");
Expand All @@ -49,18 +34,16 @@ function hasClass(elem, className) {
}

function addClass(elem, className) {
if (!elem || !elem.classList) {
return;
if (elem && elem.classList) {
elem.classList.add(className);
}
elem.classList.add(className);
}

// eslint-disable-next-line no-unused-vars
function removeClass(elem, className) {
if (!elem || !elem.classList) {
return;
if (elem && elem.classList) {
elem.classList.remove(className);
}
elem.classList.remove(className);
}

/**
Expand Down Expand Up @@ -127,11 +110,7 @@ function getCurrentValue(name) {
// Rust to the JS. If there is no such element, return null.
const getVar = (function getVar(name) {
const el = document.getElementById("rustdoc-vars");
if (el) {
return el.attributes["data-" + name].value;
} else {
return null;
}
return el ? el.attributes["data-" + name].value : null;
});

function switchTheme(newThemeName, saveTheme) {
Expand All @@ -158,6 +137,9 @@ function switchTheme(newThemeName, saveTheme) {
}

const updateTheme = (function() {
// only listen to (prefers-color-scheme: dark) because light is the default
const mql = window.matchMedia("(prefers-color-scheme: dark)");

/**
* Update the current theme to match whatever the current combination of
* * the preference for using the system theme
Expand All @@ -177,7 +159,7 @@ const updateTheme = (function() {
const lightTheme = getSettingValue("preferred-light-theme") || "light";
const darkTheme = getSettingValue("preferred-dark-theme") || "dark";

if (isDarkMode()) {
if (mql.matches) {
use(darkTheme, true);
} else {
// prefers a light theme, or has no preference
Expand All @@ -191,37 +173,7 @@ const updateTheme = (function() {
}
}

// This is always updated below to a function () => bool.
let isDarkMode;

// Determine the function for isDarkMode, and if we have
// `window.matchMedia`, set up an event listener on the preferred color
// scheme.
//
// Otherwise, fall back to the prefers-color-scheme value CSS captured in
// the "content" property.
if (window.matchMedia) {
// only listen to (prefers-color-scheme: dark) because light is the default
const mql = window.matchMedia("(prefers-color-scheme: dark)");

isDarkMode = () => mql.matches;

if (mql.addEventListener) {
mql.addEventListener("change", updateTheme);
} else {
// This is deprecated, see:
// https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/addListener
mql.addListener(updateTheme);
}
} else {
// fallback to the CSS computed value
const cssContent = getComputedStyle(document.documentElement)
.getPropertyValue("content");
// (Note: the double-quotes come from that this is a CSS value, which
// might be a length, string, etc.)
const cssColorScheme = cssContent || "\"light\"";
isDarkMode = () => (cssColorScheme === "\"dark\"");
}
mql.addEventListener("change", updateTheme);

return updateTheme;
})();
Expand Down