Rollup merge of rust-lang#104366 - GuillaumeGomez:simplify-settings-t…

…heme-choice, r=notriddle Simplify settings theme choice I removed the storage changes from rust-lang#98765 and only kept the UI changes. You can test it [here](https://rustdoc.crud.net/imperio/simplify-settings-theme-choice/foo/index.html). Discussion about this still in progress [on zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Last.20part.20of.20settings.20simplification). r? ```@notriddle```
matthiaskrgr · Nov 17, 2022 · b0b92ef · b0b92ef
2 parents d5d8eee + dacf9b8
commit b0b92ef
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 69 deletions.
diff --git a/src/librustdoc/html/static/js/settings.js b/src/librustdoc/html/static/js/settings.js
@@ -9,13 +9,16 @@
     const isSettingsPage = window.location.pathname.endsWith("/settings.html");
 
     function changeSetting(settingName, value) {
+        if (settingName === "theme") {
+            const useSystem = value === "system preference" ? "true" : "false";
+            updateLocalStorage("use-system-theme", useSystem);
+        }
         updateLocalStorage(settingName, value);
 
         switch (settingName) {
             case "theme":
             case "preferred-dark-theme":
             case "preferred-light-theme":
-            case "use-system-theme":
                 updateSystemTheme();
                 updateLightAndDark();
                 break;
@@ -45,19 +48,18 @@
     }
 
     function showLightAndDark() {
-        addClass(document.getElementById("theme").parentElement, "hidden");
         removeClass(document.getElementById("preferred-light-theme").parentElement, "hidden");
         removeClass(document.getElementById("preferred-dark-theme").parentElement, "hidden");
     }
 
     function hideLightAndDark() {
         addClass(document.getElementById("preferred-light-theme").parentElement, "hidden");
         addClass(document.getElementById("preferred-dark-theme").parentElement, "hidden");
-        removeClass(document.getElementById("theme").parentElement, "hidden");
     }
 
     function updateLightAndDark() {
-        if (getSettingValue("use-system-theme") !== "false") {
+        const useSystem = getSettingValue("use-system-theme");
+        if (useSystem === "true" || (useSystem === null && getSettingValue("theme") === null)) {
             showLightAndDark();
         } else {
             hideLightAndDark();
@@ -91,7 +93,18 @@
         });
         onEachLazy(settingsElement.querySelectorAll("input[type=\"radio\"]"), elem => {
             const settingId = elem.name;
-            const settingValue = getSettingValue(settingId);
+            let settingValue = getSettingValue(settingId);
+            if (settingId === "theme") {
+                const useSystem = getSettingValue("use-system-theme");
+                if (useSystem === "true" || settingValue === null) {
+                    if (useSystem !== "false") {
+                        settingValue = "system preference";
+                    } else {
+                        // This is the default theme.
+                        settingValue = "light";
+                    }
+                }
+            }
             if (settingValue !== null && settingValue !== "null") {
                 elem.checked = settingValue === elem.value;
             }
@@ -120,26 +133,30 @@
 
             if (setting["options"] !== undefined) {
                 // This is a select setting.
-                output += `<div class="radio-line" id="${js_data_name}">\
-                        <span class="setting-name">${setting_name}</span>\
-                        <div class="choices">`;
+                output += `\
+<div class="radio-line" id="${js_data_name}">
+    <span class="setting-name">${setting_name}</span>
+<div class="choices">`;
                 onEach(setting["options"], option => {
                     const checked = option === setting["default"] ? " checked" : "";
+                    const full = `${js_data_name}-${option.replace(/ /g,"-")}`;
 
-                    output += `<label for="${js_data_name}-${option}" class="choice">\
-                           <input type="radio" name="${js_data_name}" \
-                                id="${js_data_name}-${option}" value="${option}"${checked}>\
-                           <span>${option}</span>\
-                         </label>`;
+                    output += `\
+<label for="${full}" class="choice">
+    <input type="radio" name="${js_data_name}"
+        id="${full}" value="${option}"${checked}>
+    <span>${option}</span>
+</label>`;
                 });
                 output += "</div></div>";
             } else {
                 // This is a toggle.
                 const checked = setting["default"] === true ? " checked" : "";
-                output += `<label class="toggle">\
-                        <input type="checkbox" id="${js_data_name}"${checked}>\
-                        <span class="label">${setting_name}</span>\
-                    </label>`;
+                output += `\
+<label class="toggle">\
+    <input type="checkbox" id="${js_data_name}"${checked}>\
+    <span class="label">${setting_name}</span>\
+</label>`;
             }
             output += "</div>";
         }
@@ -156,16 +173,11 @@
         theme_names.push("light", "dark", "ayu");
 
         const settings = [
-            {
-                "name": "Use system theme",
-                "js_name": "use-system-theme",
-                "default": true,
-            },
             {
                 "name": "Theme",
                 "js_name": "theme",
-                "default": "light",
-                "options": theme_names,
+                "default": "system preference",
+                "options": theme_names.concat("system preference"),
             },
             {
                 "name": "Preferred light theme",

diff --git a/src/test/rustdoc-gui/settings.goml b/src/test/rustdoc-gui/settings.goml
@@ -37,8 +37,7 @@ click: "#settings-menu"
 wait-for: "#settings"
 
 // We check that the "Use system theme" is disabled.
-assert-property: ("#use-system-theme", {"checked": "false"})
-assert: "//*[@class='setting-line']//span[text()='Use system theme']"
+assert-property: ("#theme-system-preference", {"checked": "false"})
 // Meaning that only the "theme" menu is showing up.
 assert: ".setting-line:not(.hidden) #theme"
 assert: ".setting-line.hidden #preferred-dark-theme"
@@ -115,13 +114,6 @@ assert-css: (
         "border-color": "rgb(221, 221, 221)",
     },
 )
-assert-css: (
-    "#use-system-theme",
-    {
-        "background-color": "rgba(0, 0, 0, 0)",
-        "border-color": "rgb(221, 221, 221)",
-    }
-)
 // Let's start with the hover for toggles.
 move-cursor-to: "#auto-hide-large-items"
 assert-css: (
@@ -131,14 +123,6 @@ assert-css: (
         "border-color": "rgb(33, 150, 243)",
     },
 )
-move-cursor-to: "#use-system-theme"
-assert-css: (
-    "#use-system-theme",
-    {
-        "background-color": "rgba(0, 0, 0, 0)",
-        "border-color": "rgb(33, 150, 243)",
-    }
-)
 move-cursor-to: "#settings-menu > a"
 // Let's now check with the focus for toggles.
 focus: "#auto-hide-large-items"
@@ -150,15 +134,6 @@ assert-css: (
         "box-shadow": "rgb(33, 150, 243) 0px 0px 1px 1px",
     },
 )
-focus: "#use-system-theme"
-assert-css: (
-    "#use-system-theme",
-    {
-        "background-color": "rgba(0, 0, 0, 0)",
-        "border-color": "rgb(221, 221, 221)",
-        "box-shadow": "rgb(33, 150, 243) 0px 0px 1px 1px",
-    },
-)
 // Now we check we both focus and hover for toggles.
 move-cursor-to: "#auto-hide-large-items"
 focus: "#auto-hide-large-items"
@@ -170,24 +145,12 @@ assert-css: (
         "box-shadow": "rgb(33, 150, 243) 0px 0px 1px 1px",
     },
 )
-move-cursor-to: "#use-system-theme"
-focus: "#use-system-theme"
-assert-css: (
-    "#use-system-theme",
-    {
-        "background-color": "rgba(0, 0, 0, 0)",
-        "border-color": "rgb(33, 150, 243)",
-        "box-shadow": "rgb(33, 150, 243) 0px 0px 1px 1px",
-    },
-)
 
 // We now switch the display.
-click: "#use-system-theme"
+click: "#theme-system-preference"
 // Wait for the hidden element to show up.
 wait-for: ".setting-line:not(.hidden) #preferred-dark-theme"
 assert: ".setting-line:not(.hidden) #preferred-light-theme"
-// Check that the theme picking is hidden.
-assert: ".setting-line.hidden #theme"
 
 // We check their text as well.
 assert-text: ("#preferred-dark-theme .setting-name", "Preferred dark theme")

diff --git a/src/test/rustdoc-gui/theme-change.goml b/src/test/rustdoc-gui/theme-change.goml
@@ -2,31 +2,66 @@
 goto: "file://" + |DOC_PATH| + "/test_docs/index.html"
 local-storage: {"rustdoc-use-system-theme": "false", "rustdoc-theme": "dark"}
 reload:
+
+store-value: (background_light, "rgb(255, 255, 255)")
+store-value: (background_dark, "rgb(53, 53, 53)")
+store-value: (background_ayu, "rgb(15, 20, 25)")
+
 click: "#settings-menu"
 wait-for: "#theme-ayu"
 click: "#theme-ayu"
 // should be the ayu theme so let's check the color.
-wait-for-css: ("body", { "background-color": "rgb(15, 20, 25)" })
+wait-for-css: ("body", { "background-color": |background_ayu| })
 assert-local-storage: { "rustdoc-theme": "ayu" }
 click: "#theme-light"
 // should be the light theme so let's check the color.
-wait-for-css: ("body", { "background-color": "rgb(255, 255, 255)" })
+wait-for-css: ("body", { "background-color": |background_light| })
 assert-local-storage: { "rustdoc-theme": "light" }
 click: "#theme-dark"
 // Should be the dark theme so let's check the color.
-wait-for-css: ("body", { "background-color": "rgb(53, 53, 53)" })
+wait-for-css: ("body", { "background-color": |background_dark| })
 assert-local-storage: { "rustdoc-theme": "dark" }
 
+local-storage: {
+    "rustdoc-preferred-light-theme": "light",
+    "rustdoc-preferred-dark-theme": "light",
+}
 goto: "file://" + |DOC_PATH| + "/settings.html"
+
 wait-for: "#settings"
 click: "#theme-light"
-wait-for-css: ("body", { "background-color": "rgb(255, 255, 255)" })
+wait-for-css: ("body", { "background-color": |background_light| })
 assert-local-storage: { "rustdoc-theme": "light" }
 
 click: "#theme-dark"
-wait-for-css: ("body", { "background-color": "rgb(53, 53, 53)" })
+wait-for-css: ("body", { "background-color": |background_dark| })
 assert-local-storage: { "rustdoc-theme": "dark" }
 
 click: "#theme-ayu"
-wait-for-css: ("body", { "background-color": "rgb(15, 20, 25)" })
+wait-for-css: ("body", { "background-color": |background_ayu| })
 assert-local-storage: { "rustdoc-theme": "ayu" }
+
+assert-local-storage-false: { "rustdoc-use-system-theme": "true" }
+click: "#theme-system-preference"
+wait-for: ".setting-line:not(.hidden) #preferred-light-theme"
+assert-local-storage: { "rustdoc-use-system-theme": "true" }
+// We click on both preferred light and dark themes to be sure that there is a change.
+click: "#preferred-light-theme-dark"
+click: "#preferred-dark-theme-dark"
+wait-for-css: ("body", { "background-color": |background_dark| })
+
+reload:
+// Ensure that the "preferred themes" are still displayed.
+wait-for: ".setting-line:not(.hidden) #preferred-light-theme"
+click: "#theme-light"
+wait-for-css: ("body", { "background-color": |background_light| })
+assert-local-storage: { "rustdoc-theme": "light" }
+// Ensure it's now hidden again
+wait-for: ".setting-line.hidden #preferred-light-theme"
+// And ensure the theme was rightly set.
+wait-for-css: ("body", { "background-color": |background_light| })
+assert-local-storage: { "rustdoc-theme": "light" }
+
+reload:
+wait-for: "#settings"
+assert: ".setting-line.hidden #preferred-light-theme"