Skip to content

Commit 88756da

Browse files
nikk15nikk15
committed
Bug 1857298 - Add close button to open tabs in view r=fxview-reviewers,fluent-reviewers,desktop-theme-reviewers,bolsson,sclements
- created tertiary button option for tab rows, styled by a class prop - added close button to open tabs rows - removed close tab from open tab context menu - updated tests to reflect removed panel item in open tab context menu - added test for the new open tabs close button - added telemetry for close open tab (requires data review) - added test suite for keyboard navigation across tab row buttons Differential Revision: https://phabricator.services.mozilla.com/D200689
1 parent cba7874 commit 88756da

17 files changed

+413
-100
lines changed

‎browser/components/firefoxview/firefoxview.css

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,24 @@ body {
9292
margin: 0;
9393
}
9494

95-
fxview-tab-list.with-dismiss-button::part(secondary-button) {
96-
background-image: url("chrome://global/skin/icons/close.svg");
95+
fxview-category-button:focus-visible {
96+
outline-offset: var(--in-content-focus-outline-inset);
9797
}
9898

99-
fxview-tab-list.with-context-menu::part(secondary-button) {
100-
background-image: url("chrome://global/skin/icons/more.svg");
99+
fxview-category-button[name="recentbrowsing"]::part(icon) {
100+
background-image: url("chrome://browser/content/firefoxview/category-recentbrowsing.svg");
101+
}
102+
fxview-category-button[name="opentabs"]::part(icon) {
103+
background-image: url("chrome://browser/content/firefoxview/category-opentabs.svg");
104+
}
105+
fxview-category-button[name="recentlyclosed"]::part(icon) {
106+
background-image: url("chrome://browser/content/firefoxview/category-recentlyclosed.svg");
107+
}
108+
fxview-category-button[name="syncedtabs"]::part(icon) {
109+
background-image: url("chrome://browser/content/firefoxview/category-syncedtabs.svg");
110+
}
111+
fxview-category-button[name="history"]::part(icon) {
112+
background-image: url("chrome://browser/content/firefoxview/category-history.svg");
101113
}
102114

103115
.sticky-container {

‎browser/components/firefoxview/fxview-tab-list.mjs

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ if (!window.IS_STORYBOOK) {
4848
* @property {boolean} pinnedTabsGridView - Whether to show pinned tabs in a grid view
4949
* @property {Array} tabItems - Items to show in the tab list
5050
* @property {string} searchQuery - The query string to highlight, if provided.
51+
* @property {string} secondaryActionClass - The class used to style the secondary action element
52+
* @property {string} tertiaryActionClass - The class used to style the tertiary action element
5153
*/
5254
export default class FxviewTabList extends MozLitElement {
5355
constructor() {
@@ -79,6 +81,8 @@ export default class FxviewTabList extends MozLitElement {
7981
tabItems: { type: Array },
8082
updatesPaused: { type: Boolean },
8183
searchQuery: { type: String },
84+
secondaryActionClass: { type: String },
85+
tertiaryActionClass: { type: String },
8286
};
8387

8488
static queries = {
@@ -244,8 +248,16 @@ export default class FxviewTabList extends MozLitElement {
244248
this.currentActiveElementId === "fxview-tab-row-main"
245249
) {
246250
this.currentActiveElementId = fxviewTabRow.focusMediaButton();
247-
} else {
248-
this.currentActiveElementId = fxviewTabRow.focusButton();
251+
} else if (
252+
this.currentActiveElementId === "fxview-tab-row-media-button" ||
253+
this.currentActiveElementId === "fxview-tab-row-main"
254+
) {
255+
this.currentActiveElementId = fxviewTabRow.focusSecondaryButton();
256+
} else if (
257+
fxviewTabRow.tertiaryButtonEl &&
258+
this.currentActiveElementId === "fxview-tab-row-secondary-button"
259+
) {
260+
this.currentActiveElementId = fxviewTabRow.focusTertiaryButton();
249261
}
250262
}
251263

@@ -257,6 +269,10 @@ export default class FxviewTabList extends MozLitElement {
257269
this.activeIndex === this.pinnedTabs.length))
258270
) {
259271
this.focusPrevRow();
272+
} else if (
273+
this.currentActiveElementId === "fxview-tab-row-tertiary-button"
274+
) {
275+
this.currentActiveElementId = fxviewTabRow.focusSecondaryButton();
260276
} else if (
261277
(fxviewTabRow.indicators?.includes("soundplaying") ||
262278
fxviewTabRow.indicators?.includes("muted")) &&
@@ -354,6 +370,10 @@ export default class FxviewTabList extends MozLitElement {
354370
: "listitem"}
355371
.secondaryL10nId=${tabItem.secondaryL10nId}
356372
.secondaryL10nArgs=${ifDefined(tabItem.secondaryL10nArgs)}
373+
.tertiaryL10nId=${ifDefined(tabItem.tertiaryL10nId)}
374+
.tertiaryL10nArgs=${ifDefined(tabItem.tertiaryL10nArgs)}
375+
.secondaryActionClass=${this.secondaryActionClass}
376+
.tertiaryActionClass=${ifDefined(this.tertiaryActionClass)}
357377
.sourceClosedId=${ifDefined(tabItem.sourceClosedId)}
358378
.sourceWindowId=${ifDefined(tabItem.sourceWindowId)}
359379
.closedId=${ifDefined(tabItem.closedId || tabItem.closedId)}
@@ -456,6 +476,10 @@ customElements.define("fxview-tab-list", FxviewTabList);
456476
* @property {string} primaryL10nArgs - The l10n args used for the primary action element
457477
* @property {string} secondaryL10nId - The l10n id used for the secondary action button
458478
* @property {string} secondaryL10nArgs - The l10n args used for the secondary action element
479+
* @property {string} secondaryActionClass - The class used to style the secondary action element
480+
* @property {string} tertiaryL10nId - The l10n id used for the tertiary action button
481+
* @property {string} tertiaryL10nArgs - The l10n args used for the tertiary action element
482+
* @property {string} tertiaryActionClass - The class used to style the tertiary action element
459483
* @property {object} tabElement - The MozTabbrowserTab element for the tab item.
460484
* @property {number} time - The timestamp for when the tab was last accessed.
461485
* @property {string} title - The title for the tab item.
@@ -484,6 +508,10 @@ export class FxviewTabRow extends MozLitElement {
484508
primaryL10nArgs: { type: String },
485509
secondaryL10nId: { type: String },
486510
secondaryL10nArgs: { type: String },
511+
secondaryActionClass: { type: String },
512+
tertiaryL10nId: { type: String },
513+
tertiaryL10nArgs: { type: String },
514+
tertiaryActionClass: { type: String },
487515
closedId: { type: Number },
488516
sourceClosedId: { type: Number },
489517
sourceWindowId: { type: String },
@@ -497,7 +525,8 @@ export class FxviewTabRow extends MozLitElement {
497525

498526
static queries = {
499527
mainEl: "#fxview-tab-row-main",
500-
buttonEl: "#fxview-tab-row-secondary-button:not([hidden])",
528+
secondaryButtonEl: "#fxview-tab-row-secondary-button:not([hidden])",
529+
tertiaryButtonEl: "#fxview-tab-row-tertiary-button",
501530
mediaButtonEl: "#fxview-tab-row-media-button",
502531
pinnedTabButtonEl: "button#fxview-tab-row-main",
503532
};
@@ -536,9 +565,14 @@ export class FxviewTabRow extends MozLitElement {
536565
this.currentFocusable.focus();
537566
}
538567

539-
focusButton() {
540-
this.buttonEl.focus();
541-
return this.buttonEl.id;
568+
focusSecondaryButton() {
569+
this.secondaryButtonEl.focus();
570+
return this.secondaryButtonEl.id;
571+
}
572+
573+
focusTertiaryButton() {
574+
this.tertiaryButtonEl.focus();
575+
return this.tertiaryButtonEl.id;
542576
}
543577

544578
focusMediaButton() {
@@ -667,6 +701,23 @@ export class FxviewTabRow extends MozLitElement {
667701
}
668702
}
669703

704+
tertiaryActionHandler(event) {
705+
if (
706+
(event.type == "click" && event.detail && !event.altKey) ||
707+
// detail=0 is from keyboard
708+
(event.type == "click" && !event.detail)
709+
) {
710+
event.preventDefault();
711+
this.dispatchEvent(
712+
new CustomEvent("fxview-tab-list-tertiary-action", {
713+
bubbles: true,
714+
composed: true,
715+
detail: { originalEvent: event, item: this },
716+
})
717+
);
718+
}
719+
}
720+
670721
muteOrUnmuteTab() {
671722
this.tabElement.toggleMuteAudio();
672723
}
@@ -838,9 +889,14 @@ export class FxviewTabRow extends MozLitElement {
838889
${when(
839890
this.secondaryL10nId && this.secondaryActionHandler,
840891
() => html`<button
841-
class="fxview-tab-row-button ghost-button icon-button semi-transparent"
892+
class=${classMap({
893+
"fxview-tab-row-button": true,
894+
"ghost-button": true,
895+
"icon-button": true,
896+
"semi-transparent": true,
897+
[this.secondaryActionClass]: this.secondaryActionClass,
898+
})}
842899
id="fxview-tab-row-secondary-button"
843-
part="secondary-button"
844900
data-l10n-id=${this.secondaryL10nId}
845901
data-l10n-args=${ifDefined(this.secondaryL10nArgs)}
846902
aria-haspopup=${ifDefined(this.hasPopup)}
@@ -877,6 +933,27 @@ export class FxviewTabRow extends MozLitElement {
877933
this.#pinnedTabItemTemplate.bind(this),
878934
this.#unpinnedTabItemTemplate.bind(this)
879935
)}
936+
${when(
937+
this.tertiaryL10nId && this.tertiaryActionHandler,
938+
() => html`<button
939+
class=${classMap({
940+
"fxview-tab-row-button": true,
941+
"ghost-button": true,
942+
"icon-button": true,
943+
"semi-transparent": true,
944+
[this.tertiaryActionClass]: this.tertiaryActionClass,
945+
})}
946+
id="fxview-tab-row-tertiary-button"
947+
data-l10n-id=${this.tertiaryL10nId}
948+
data-l10n-args=${ifDefined(this.tertiaryL10nArgs)}
949+
aria-haspopup=${ifDefined(this.hasPopup)}
950+
@click=${this.tertiaryActionHandler}
951+
tabindex="${this.active &&
952+
this.currentActiveElementId === "fxview-tab-row-tertiary-button"
953+
? "0"
954+
: "-1"}"
955+
></button>`
956+
)}
880957
`;
881958
}
882959

‎browser/components/firefoxview/fxview-tab-row.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,14 @@
200200
&[soundplaying="true"] {
201201
background-image: url("chrome://global/skin/media/audio.svg");
202202
}
203+
204+
&.dismiss-button {
205+
background-image: url("chrome://global/skin/icons/close.svg");
206+
}
207+
208+
&.options-button {
209+
background-image: url("chrome://global/skin/icons/more.svg");
210+
}
203211
}
204212

205213
@media (prefers-contrast) {

‎browser/components/firefoxview/history.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ class HistoryInView extends ViewPage {
418418
></h3>
419419
<fxview-tab-list
420420
slot="main"
421-
class="with-context-menu"
421+
secondaryActionClass="options-button"
422422
dateTimeFormat=${historyItem.l10nId.includes("prev-month")
423423
? "dateTime"
424424
: "time"}

‎browser/components/firefoxview/opentabs.mjs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,18 @@ class OpenTabsInViewCard extends ViewPageContent {
505505
}
506506
}
507507

508+
closeTab(event) {
509+
const tab = event.originalTarget.tabElement;
510+
tab?.ownerGlobal.gBrowser.removeTab(tab);
511+
512+
Services.telemetry.recordEvent(
513+
"firefoxview_next",
514+
"close_open_tab",
515+
"tabs",
516+
null
517+
);
518+
}
519+
508520
viewVisibleCallback() {
509521
this.getRootNode().host.toggleVisibilityInCardContainer(true);
510522
}
@@ -539,11 +551,13 @@ class OpenTabsInViewCard extends ViewPageContent {
539551
)}
540552
<div class="fxview-tab-list-container" slot="main">
541553
<fxview-tab-list
542-
class="with-context-menu"
543554
.hasPopup=${"menu"}
544555
?compactRows=${this.classList.contains("width-limited")}
545556
@fxview-tab-list-primary-action=${this.onTabListRowClick}
546557
@fxview-tab-list-secondary-action=${this.openContextMenu}
558+
@fxview-tab-list-tertiary-action=${this.closeTab}
559+
secondaryActionClass="options-button"
560+
tertiaryActionClass="dismiss-button"
547561
.maxTabsLength=${this.getMaxTabsLength()}
548562
.tabItems=${this.searchResults ||
549563
getTabListItems(this.tabs, this.recentBrowsing)}
@@ -780,11 +794,6 @@ class OpenTabsContextMenu extends MozLitElement {
780794
href="chrome://browser/content/firefoxview/firefoxview.css"
781795
/>
782796
<panel-list data-tab-type="opentabs">
783-
<panel-item
784-
data-l10n-id="fxviewtabrow-close-tab"
785-
data-l10n-attrs="accesskey"
786-
@click=${this.closeTab}
787-
></panel-item>
788797
<panel-item
789798
data-l10n-id="fxviewtabrow-move-tab"
790799
data-l10n-attrs="accesskey"
@@ -947,6 +956,14 @@ function getTabListItems(tabs, isRecentBrowsing) {
947956
isRecentBrowsing || (!isRecentBrowsing && !tab.pinned)
948957
? JSON.stringify({ tabTitle: tab.label })
949958
: null,
959+
tertiaryL10nId:
960+
isRecentBrowsing || (!isRecentBrowsing && !tab.pinned)
961+
? "fxviewtabrow-close-tab-button"
962+
: null,
963+
tertiaryL10nArgs:
964+
isRecentBrowsing || (!isRecentBrowsing && !tab.pinned)
965+
? JSON.stringify({ tabTitle: tab.label })
966+
: null,
950967
tabElement: tab,
951968
time: tab.lastAccessed,
952969
title: tab.label,

‎browser/components/firefoxview/recentlyclosed.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ class RecentlyClosedTabsInView extends ViewPage {
398398
.tabItems=${this.searchResults || this.recentlyClosedTabs}
399399
@fxview-tab-list-secondary-action=${this.onDismissTab}
400400
@fxview-tab-list-primary-action=${this.onReopenTab}
401+
secondaryActionClass="dismiss-button"
401402
></fxview-tab-list>
402403
`
403404
)}

‎browser/components/firefoxview/syncedtabs.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ class SyncedTabsInView extends ViewPage {
426426
maxTabsLength=${this.showAll ? -1 : this.maxTabsLength}
427427
@fxview-tab-list-primary-action=${this.onOpenLink}
428428
@fxview-tab-list-secondary-action=${this.onContextMenu}
429+
secondaryActionClass="options-button"
429430
>
430431
${this.panelListTemplate()}
431432
</fxview-tab-list>`;

‎browser/components/firefoxview/tests/browser/browser.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,6 @@ fail-if = ["a11y_checks"] # Bug 1850591 clicked moz-page-nav-button button is no
8484

8585
["browser_tab_close_last_tab.js"]
8686

87+
["browser_tab_list_keyboard_navigation.js"]
88+
8789
["browser_tab_on_close_warning.js"]

‎browser/components/firefoxview/tests/browser/browser_firefoxview_general_telemetry.js

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,11 @@ add_task(async function test_context_menu_new_window_telemetry() {
198198
let firstTabList = historyComponent.lists[0];
199199
let firstItem = firstTabList.rowEls[0];
200200
let panelList = historyComponent.panelList;
201-
EventUtils.synthesizeMouseAtCenter(firstItem.buttonEl, {}, content);
201+
EventUtils.synthesizeMouseAtCenter(
202+
firstItem.secondaryButtonEl,
203+
{},
204+
content
205+
);
202206
await BrowserTestUtils.waitForEvent(panelList, "shown");
203207
await clearAllParentTelemetryEvents();
204208
let panelItems = Array.from(panelList.children).filter(
@@ -254,14 +258,22 @@ add_task(async function test_context_menu_private_window_telemetry() {
254258
let firstTabList = historyComponent.lists[0];
255259
let firstItem = firstTabList.rowEls[0];
256260
let panelList = historyComponent.panelList;
257-
EventUtils.synthesizeMouseAtCenter(firstItem.buttonEl, {}, content);
261+
EventUtils.synthesizeMouseAtCenter(
262+
firstItem.secondaryButtonEl,
263+
{},
264+
content
265+
);
258266
await BrowserTestUtils.waitForEvent(panelList, "shown");
259267
await clearAllParentTelemetryEvents();
260268
let panelItems = Array.from(panelList.children).filter(
261269
panelItem => panelItem.nodeName === "PANEL-ITEM"
262270
);
263271

264-
EventUtils.synthesizeMouseAtCenter(firstItem.buttonEl, {}, content);
272+
EventUtils.synthesizeMouseAtCenter(
273+
firstItem.secondaryButtonEl,
274+
{},
275+
content
276+
);
265277
info("Context menu button clicked.");
266278
await BrowserTestUtils.waitForEvent(panelList, "shown");
267279
info("Context menu shown.");
@@ -323,14 +335,22 @@ add_task(async function test_context_menu_delete_from_history_telemetry() {
323335
let firstTabList = historyComponent.lists[0];
324336
let firstItem = firstTabList.rowEls[0];
325337
let panelList = historyComponent.panelList;
326-
EventUtils.synthesizeMouseAtCenter(firstItem.buttonEl, {}, content);
338+
EventUtils.synthesizeMouseAtCenter(
339+
firstItem.secondaryButtonEl,
340+
{},
341+
content
342+
);
327343
await BrowserTestUtils.waitForEvent(panelList, "shown");
328344
await clearAllParentTelemetryEvents();
329345
let panelItems = Array.from(panelList.children).filter(
330346
panelItem => panelItem.nodeName === "PANEL-ITEM"
331347
);
332348

333-
EventUtils.synthesizeMouseAtCenter(firstItem.buttonEl, {}, content);
349+
EventUtils.synthesizeMouseAtCenter(
350+
firstItem.secondaryButtonEl,
351+
{},
352+
content
353+
);
334354
info("Context menu button clicked.");
335355
await BrowserTestUtils.waitForEvent(panelList, "shown");
336356
info("Context menu shown.");

0 commit comments

Comments
 (0)