commit fe1d8d747881630cf53a62a30a013ccdfc854fc2
parent fe9a7f0ec0e7364f02813905230f8b3808e9b4e9
Author: Erik Nordin <enordin@mozilla.com>
Date: Thu, 18 Dec 2025 01:17:28 +0000
Bug 2002127 - Part 17: Update Ghost Button Usage r=translations-reviewers,hjones
This commit updates the usage of ghost icon buttons in the Translations Settings
subpage to only be used in the case of showing a loading spinner, a warning icon,
or an error icon. If the button is intended to be clicked, whether enabled or disabled,
it will be shown as a non-ghost button.
Differential Revision: https://phabricator.services.mozilla.com/D274829
Diffstat:
5 files changed, 112 insertions(+), 37 deletions(-)
diff --git a/browser/components/preferences/main.js b/browser/components/preferences/main.js
@@ -4035,7 +4035,7 @@ SettingGroupManager.registerGroups({
control: "moz-button",
controlAttrs: {
slot: "actions",
- type: "ghost",
+ type: "icon",
iconsrc: "chrome://browser/skin/downloads/downloads.svg",
},
},
diff --git a/browser/components/preferences/translations.js b/browser/components/preferences/translations.js
@@ -969,7 +969,7 @@ const TranslationsSettings = {
const removeButton = document.createElement("moz-button");
removeButton.setAttribute("slot", "actions-start");
- removeButton.setAttribute("type", "icon ghost");
+ removeButton.setAttribute("type", "icon");
removeButton.setAttribute(
"iconsrc",
"chrome://global/skin/icons/delete.svg"
@@ -1254,7 +1254,7 @@ const TranslationsSettings = {
const removeButton = document.createElement("moz-button");
removeButton.setAttribute("slot", "actions-start");
- removeButton.setAttribute("type", "icon ghost");
+ removeButton.setAttribute("type", "icon");
removeButton.setAttribute(
"iconsrc",
"chrome://global/skin/icons/delete.svg"
@@ -1416,7 +1416,7 @@ const TranslationsSettings = {
for (const origin of sortedOrigins) {
const removeButton = document.createElement("moz-button");
removeButton.setAttribute("slot", "actions-start");
- removeButton.setAttribute("type", "icon ghost");
+ removeButton.setAttribute("type", "icon");
removeButton.setAttribute(
"iconsrc",
"chrome://global/skin/icons/delete.svg"
@@ -1601,6 +1601,22 @@ const TranslationsSettings = {
},
/**
+ * Toggle ghost styling on icon buttons.
+ *
+ * @param {HTMLElement|null} button
+ * @param {boolean} isGhost
+ */
+ setIconButtonGhostState(button, isGhost) {
+ if (!button) {
+ return;
+ }
+ const type = isGhost ? "icon ghost" : "icon";
+ if (button.getAttribute("type") !== type) {
+ button.setAttribute("type", type);
+ }
+ },
+
+ /**
* Reset the download dropdown back to its placeholder value.
*/
resetDownloadSelect() {
@@ -1670,12 +1686,13 @@ const TranslationsSettings = {
async createDeleteConfirmationItem(langTag, item) {
const warningButton = document.createElement("moz-button");
warningButton.setAttribute("slot", "actions-start");
- warningButton.setAttribute("type", "icon ghost");
+ warningButton.setAttribute("type", "icon");
warningButton.setAttribute("iconsrc", DOWNLOAD_WARNING_ICON);
warningButton.style.pointerEvents = "none";
warningButton.style.color = "var(--icon-color-warning)";
warningButton.classList.add(DOWNLOAD_LANGUAGE_REMOVE_BUTTON_CLASS);
warningButton.dataset.langTag = langTag;
+ this.setIconButtonGhostState(warningButton, true);
const sizeLabel = this.formatLanguageSize(langTag) ?? "0";
const languageLabel = this.formatLanguageLabel(langTag) ?? langTag;
@@ -1726,12 +1743,13 @@ const TranslationsSettings = {
async createFailedDownloadItem(langTag, item) {
const errorButton = document.createElement("moz-button");
errorButton.setAttribute("slot", "actions-start");
- errorButton.setAttribute("type", "icon ghost");
+ errorButton.setAttribute("type", "icon");
errorButton.setAttribute("iconsrc", DOWNLOAD_ERROR_ICON);
errorButton.style.pointerEvents = "none";
errorButton.style.color = "var(--text-color-error)";
errorButton.classList.add(DOWNLOAD_LANGUAGE_REMOVE_BUTTON_CLASS);
errorButton.dataset.langTag = langTag;
+ this.setIconButtonGhostState(errorButton, true);
const sizeLabel = this.formatLanguageSize(langTag) ?? "0";
const languageLabel = this.formatLanguageLabel(langTag) ?? langTag;
@@ -1785,7 +1803,7 @@ const TranslationsSettings = {
const removeButton = document.createElement("moz-button");
removeButton.setAttribute("slot", "actions-start");
- removeButton.setAttribute("type", "icon ghost");
+ removeButton.setAttribute("type", "icon");
removeButton.setAttribute(
"iconsrc",
isDownloading ? DOWNLOAD_LOADING_ICON : DOWNLOAD_DELETE_ICON
@@ -1796,6 +1814,11 @@ const TranslationsSettings = {
if (isDownloading) {
removeButton.style.pointerEvents = "none";
}
+ this.setIconButtonGhostState(
+ removeButton,
+ isDownloading ||
+ removeButton.getAttribute("iconsrc") === DOWNLOAD_LOADING_ICON
+ );
item.setAttribute("label", label);
if (isDownloading) {
diff --git a/browser/components/translations/tests/browser/browser_translations_about_settings_subpage_download_langs_basic.js b/browser/components/translations/tests/browser/browser_translations_about_settings_subpage_download_langs_basic.js
@@ -67,37 +67,49 @@ add_task(async function test_download_languages_basic_flow() {
]);
info("Download French language models");
- const downloadEvents = [
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events.DownloadButtonDisabled
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events.DownloadStarted,
- { expectedDetail: { langTag: "fr" } }
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events.DownloadedLanguagesRendered,
- { expectedDetail: { languages: ["fr"], count: 1, downloading: ["fr"] } }
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events
- .DownloadedLanguagesSelectOptionsUpdated
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events.DownloadCompleted,
- { expectedDetail: { langTag: "fr" } }
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events.DownloadedLanguagesRendered,
- { expectedDetail: { languages: ["fr"], count: 1, downloading: [] } }
- ),
- translationsSettingsTestUtils.waitForEvent(
- TranslationsSettingsTestUtils.Events
- .DownloadedLanguagesSelectOptionsUpdated
- ),
- ];
+ const downloadButtonDisabled = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadButtonDisabled
+ );
+ const downloadStarted = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadStarted,
+ { expectedDetail: { langTag: "fr" } }
+ );
+ const renderDownloading = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadedLanguagesRendered,
+ { expectedDetail: { languages: ["fr"], count: 1, downloading: ["fr"] } }
+ );
+ const optionsDuringDownload = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadedLanguagesSelectOptionsUpdated
+ );
await click(downloadButton, "Start French download");
+ await Promise.all([
+ downloadButtonDisabled,
+ downloadStarted,
+ renderDownloading,
+ optionsDuringDownload,
+ ]);
+
+ const spinnerButton =
+ translationsSettingsTestUtils.getDownloadRemoveButton("fr");
+ ok(spinnerButton, "Spinner button should be present while downloading");
+ is(
+ spinnerButton.getAttribute("type"),
+ "icon ghost",
+ "Spinner button should use ghost styling while downloading"
+ );
+
+ const downloadCompleted = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadCompleted,
+ { expectedDetail: { langTag: "fr" } }
+ );
+ const renderDownloaded = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadedLanguagesRendered,
+ { expectedDetail: { languages: ["fr"], count: 1, downloading: [] } }
+ );
+ const optionsAfterDownload = translationsSettingsTestUtils.waitForEvent(
+ TranslationsSettingsTestUtils.Events.DownloadedLanguagesSelectOptionsUpdated
+ );
Assert.deepEqual(
await remoteClients.translationModels.resolvePendingDownloads(
expectedModelDownloads.length
@@ -105,13 +117,25 @@ add_task(async function test_download_languages_basic_flow() {
expectedModelDownloads,
"French models were downloaded."
);
- await Promise.all(downloadEvents);
+ await Promise.all([
+ downloadCompleted,
+ renderDownloaded,
+ optionsAfterDownload,
+ ]);
await translationsSettingsTestUtils.assertDownloadedLanguages({
languages: ["fr"],
downloading: [],
count: 1,
});
+ const removeButton =
+ translationsSettingsTestUtils.getDownloadRemoveButton("fr");
+ ok(removeButton, "Delete icon should be present after download completes");
+ is(
+ removeButton.getAttribute("type"),
+ "icon",
+ "Delete icon should not use ghost styling after download completes"
+ );
ok(frenchOption.disabled, "French option disabled after download");
info("Open delete confirmation then cancel");
@@ -123,6 +147,11 @@ add_task(async function test_download_languages_basic_flow() {
warningButton.getAttribute("iconsrc")?.includes("warning"),
"Warning icon should use warning asset"
);
+ is(
+ warningButton.getAttribute("type"),
+ "icon ghost",
+ "Warning icon should use ghost styling during delete confirmation"
+ );
const cancelRender = translationsSettingsTestUtils.waitForEvent(
TranslationsSettingsTestUtils.Events.DownloadedLanguagesRendered,
{ expectedDetail: { languages: ["fr"], count: 1, downloading: [] } }
@@ -142,6 +171,11 @@ add_task(async function test_download_languages_basic_flow() {
deleteIcon.getAttribute("iconsrc")?.includes("delete"),
"Delete icon should use delete asset"
);
+ is(
+ deleteIcon.getAttribute("type"),
+ "icon",
+ "Delete icon should not use ghost styling after canceling delete"
+ );
info("Confirm deletion after second attempt");
const deleted = translationsSettingsTestUtils.waitForEvent(
diff --git a/browser/components/translations/tests/browser/browser_translations_about_settings_subpage_download_langs_errors.js b/browser/components/translations/tests/browser/browser_translations_about_settings_subpage_download_langs_errors.js
@@ -28,6 +28,11 @@ add_task(
translationsSettingsTestUtils.getDownloadRetryButton("fr");
ok(errorButton, "Error icon should be visible");
ok(retryButton, "Retry button should be visible");
+ is(
+ errorButton.getAttribute("type"),
+ "icon ghost",
+ "Error icon should use ghost styling"
+ );
const errorMessage = getByL10nId(
"settings-translations-subpage-download-error",
document
@@ -101,6 +106,11 @@ add_task(async function test_download_error_retry_via_retry_button() {
translationsSettingsTestUtils.getDownloadRetryButton("es");
ok(errorButton, "Error icon should be visible");
ok(retryButton, "Retry button should be visible");
+ is(
+ errorButton.getAttribute("type"),
+ "icon ghost",
+ "Error icon should use ghost styling"
+ );
const errorMessage = getByL10nId(
"settings-translations-subpage-download-error",
document
diff --git a/toolkit/components/translations/tests/browser/shared-head.js b/toolkit/components/translations/tests/browser/shared-head.js
@@ -900,6 +900,14 @@ class TranslationsSettingsTestUtils {
);
await Promise.all([started, renderInProgress, optionsUpdated]);
+ const spinnerButton = this.getDownloadRemoveButton(langTag);
+ ok(spinnerButton, "Spinner button should be visible while downloading");
+ is(
+ spinnerButton.getAttribute("type"),
+ "icon ghost",
+ "Spinner button should use ghost styling while downloading"
+ );
+
const failed = this.waitForEvent(
TranslationsSettingsTestUtils.Events.DownloadFailed,
{ expectedDetail: { langTag } }
