Closed Bug 1787945 Opened 2 years ago Closed 2 years ago

As a user I want the ability to delete items listed in Firefox View (e.g. Recently Closed)

Categories

(Firefox :: Firefox View, enhancement, P3)

Firefox 106
enhancement
Points:
5

Tracking

()

RESOLVED FIXED
109 Branch
Tracking Status
firefox106 --- wontfix
firefox109 --- fixed

People

(Reporter: jchaupitre, Assigned: kcochrane)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fidefe-2022-mr1-firefox-view] )

Attachments

(4 files)

We don't have a way for users to delete entries from Firefox View.

Blocks: firefox-view

This bug description is a bit terse. The slack discussion suggested that clearing recent history doesn't work. The code suggests otherwise (ie we listen for an observer topic that should be fired by clearing recent history, and we even have automated tests that would fail if this weren't the case).

How exactly did you clear history?

Flags: needinfo?(jchaupitre)

I saw an entry in my list of last closed tab in firefox view that I didn't want to see listed here. I went ahead and deleted that particular entry from the history (Menu > History > Manage History > right click > delete). I noticed that it had not been removed from firefox view. I closed the browser and re-opened it but it had no effect. I went on with my day and saw the list in Fx View had changed some time later and could not appreciate what actions I did caused to change the list.

Flags: needinfo?(jchaupitre)
Priority: -- → P3

I'm marking this to be added to the Backlog. I think it's something we can consider in future discussions for a follow up iteration (i.e. adding ability to remove closed tab history for a single url entry in the "Recently closed list").

(In reply to Julien Chaupitre from comment #2)

I saw an entry in my list of last closed tab in firefox view that I didn't want to see listed here. I went ahead and deleted that particular entry from the history (Menu > History > Manage History > right click > delete). I noticed that it had not been removed from firefox view. I closed the browser and re-opened it but it had no effect. I went on with my day and saw the list in Fx View had changed some time later and could not appreciate what actions I did caused to change the list.

So fwiw we have too many ways to clear history and this one only deletes a single history visit entry (there could easily be more than 1 for a given URL). The more thorough option is "forget about this site" (or even "clear recent history" to forget the last N minutes/hours/days of browsing), which does clear all tabs from that site from recently closed tabs.

The other workaround is opening a new window, moving any tabs you care about to the new window, and closing the old one.

Blocks: 1788692
No longer blocks: firefox-view
Blocks: firefox-view

Some technical details for this feature: remove a dismissed tab from SessionStore, so that its reflected on all recently closed tab access points. Add the underline to the title as well as the hover states and the x dismiss button per the figma. Use telemetry to collect data for whenever a user dismisses a recently closed tab (only on volume, position is not important).

Points: --- → 5
Assignee: nobody → kcochrane
Status: NEW → ASSIGNED

Discussion on expected a11y behavior: https://mozilla.slack.com/archives/C4E0W8B8E/p1668118551293889
As discussed in the linked thread, both the dismiss button and the remaining content in the list item will be focusable. The dismiss button should be announced as "Close [tab title], button".

Attachment #9302953 - Attachment description: WIP: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=sfoster → WIP: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=sfoster,ayeddi

After discussion with Josh Berman, we've decided to use "Dismiss [tab title]" as the aria-label for the new dismiss button being added here. "Close" implies that the tab/link is currently open which is not the case. It was previously closed, and now you can optionally dismiss the tab from the list of recently closed tabs.

Attachment #9302953 - Attachment description: WIP: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=sfoster,ayeddi → WIP: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi!
Attachment #9302953 - Attachment description: WIP: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi! → Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi!
Attachment #9302953 - Attachment description: Bug 1787945 - Add delete functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi! → Bug 1787945 - Add dismiss functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi!
Attachment #9302953 - Attachment description: Bug 1787945 - Add dismiss functionality to recently closed tabs list items in Fx View r=Gijs!,ayeddi! → Bug 1787945 - Add dismiss functionality to recently closed tabs list items in Fx View r=sfoster!,ayeddi!

As previously discussed with :ayeddi, after making the updates to add the dismiss button within each list item, keyboard navigation will work as follows:
"when the user tabs to the Recently Closed Tabs list, the focus lands on a link within the fist list item, then Tab to the Close button within the same list item, then Tab to a link within the second list item, etc. Arrows Up/Down won’t move the focus between list items anymore."

Sam has brought up some concerns with the sizing of the X icon we've used for the dismiss button:
"However, 8px icons aren't really a thing we've ever done before. When we did a round of icon consolidation recently, we agreed to 3 sizes: normal/default is on a 16x16 grid ({name}.svg), , large is 20x20 ({name-20}.svg), small is 12x12 ({name}-12.svg). There are a couple of oddball exceptions, but I don't see a strong reason for this to be one. This is a variation of the close.svg, in toolkit/themes/shared/icons/close.svg, and available at chrome://global/skin/icons/close.svg. The tab close button uses this default size close.svg, do you think we could use that here as well?"
Can you let me know what you think, Josh? And Jane, could you please make sure we’re documenting these components as requested by Aaron Benson? Thanks, all!

Flags: needinfo?(jwolf)
Flags: needinfo?(jberman)
Attached image 16x16 icon (deleted) —
Attached image 8x8 icon (deleted) —

I've attached screenshots of the new dismiss button with an 8x8 icon (as aligned with the current Figma spec) and with the proposed 16x16 default "close" icon.

Hey, the icon was actually the 12px from the design system, but I for some reason had detached it from the component so it lost the padding it usually receives. I replaced the detached icon with the component and it's now 12px X 12px. If this doesnt match what exists in our dev environment though, maybe i need to increase the size.

Josh, am I looking in the right spot in the file? All the svg icons still appear to be 8x8 for me. I need to export out just the svg icon itself as either 12x12 or 16x16, but yeah 12x12 works fine for me.

Ok, so I think we need design system to weigh in. In our "12x12" icon set not all icons are exactly 12x12. Many icons have a bit of a funny aspect ratio so from the design side 12x12 more means a set of icons that fit in a 12x12 container and all have a similar weight. So whereas this close button is definitely 8x8, it's in the 12x12 set because for example, our 12x12 chevrons are actually about 4.5x8 and search is 10x10. I think we need Design Systems input here and we also need to understand when we say that "8px icons arent a thing we've done before" if that means when we design using the 12x12 icon set thats not really what ends up in the dev environment. Thanks!

Flags: needinfo?(jberman)

So I'm taking a look at this further. Our 12x12 icon is an 8x8px icon inside of a bounding box. Are we able to just use the existing icon for this and export it out?

Thank you!

Flags: needinfo?(jwolf)

Sam, could you weigh in on this given comment 16 and comment 17? It looks like most of our "12x12" icons in Figma are actually smaller than 12x12 within a 12x12 bounding box?

Flags: needinfo?(sfoster)

(In reply to Kelly Cochrane [:kcochrane] from comment #18)

Sam, could you weigh in on this given comment 16 and comment 17? It looks like most of our "12x12" icons in Figma are actually smaller than 12x12 within a 12x12 bounding box?

Yeah the 12x12 / 16x16 is the grid the icon is drawn on and doesn't necessarily dictate it should occupy all that space edge to edge. We never had a need for a 12x12 close.svg so there's no such file yet in codebase. Creating the file wasn't the problem though. I was hoping to get a design guideline for which size to use where. Those smaller icons are currently used exclusively where space is particularly tight and/or the icon is a badge or other ornament. We have neither constraint in the firefox view list items. Maybe the question is why wouldn't we use the normal 16x16 icon here? Why is this case different from every other case where we need an "X" button? That includes dismiss buttons for UI on about:newtab, all our onboarding message containers, every dialog close button.. the list goes on and on. I'm not trying to dictate our design aesthetic, I'm just frequently asked to review patches that make use of our icons and this case throws the current logic into question. If something about this dismiss button requires a smaller icon, what specifically is that requirement and how will I know when to use the small icon vs. the normal size one in the future?

Flags: needinfo?(sfoster)

I updated the figma file to use 16x16px to align with larger Fx ecosystem based on Sam's recommendation and based on additional feedback from a11y team that going with the default icon size here would be preferred.

Pushed by kcochrane@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/47632dabdc1f Add dismiss functionality to recently closed tabs list items in Fx View r=ayeddi,sfoster
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
Regressions: 1803763
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: