Stacked groups should have a close tab close button on the top tab

VERIFIED FIXED in Firefox 4.0

Status

P2
normal
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: iangilman, Assigned: seanedunn)

Tracking

unspecified
Firefox 4.0

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Place several tabs in a group. Reduce size of the group until you have a stack. Click on the stack expander. Note that none of the tabs have an "x" in the corner. 

Now remove a tab from the stack (still in expanded view). Note that the tab still does not have a close button and that clicking in the close area only expands the tab.
Priority: -- → P3
(Reporter)

Comment 1

8 years ago
Tabs should definitely have close boxes in an expanded stack, or if they get pulled out from a stack.

On a related note, there's some disagreement about whether they should have close boxes when in a collapsed stack. I don't think they should (too busy, too easy to accidentally close, potentially without even noticing, if the thumbnails are similar), whereas Kevin thinks they should (needless loss of functionality, creates a weird state where you can drag items but not close them). 

Aza, comment?
(Reporter)

Updated

8 years ago
Assignee: nobody → seanedunn
(In reply to comment #1)
> On a related note, there's some disagreement about whether they should have
> close boxes when in a collapsed stack. I don't think they should (too busy, too
> easy to accidentally close, potentially without even noticing, if the
> thumbnails are similar)

While my name is not Aza, I agree with this.
(In reply to comment #1)
> On a related note, there's some disagreement about whether they should have
> close boxes when in a collapsed stack. I don't think they should (too busy, too
> easy to accidentally close, potentially without even noticing, if the
> thumbnails are similar), whereas Kevin thinks they should (needless loss of
> functionality, creates a weird state where you can drag items but not close
> them). 

I don't understand.... When tabs are in a collapsed stack, only the top tab is visible, and all of the tabs are turned one way or another. Why would anyone expect there to be close buttons on any of them? And why would you be able to drag from a closed stack, either?

Or am I misunderstanding something?

Updated

8 years ago
Blocks: 597763

Updated

8 years ago
Priority: P3 → P2
Target Milestone: --- → Firefox 4.0

Comment 4

8 years ago
Yes, you should be able to close the top element in a stack.
Summary: Stacked groups remove tab close buttons → Stacked groups should have a close tab close button on the top tab
(Assignee)

Comment 5

8 years ago
Created attachment 479689 [details] [diff] [review]
v1

Close button stays 100% opaque with no fade for stacked tabitems.
Attachment #479689 - Flags: ui-review?
Attachment #479689 - Flags: feedback?(ian)
(Assignee)

Updated

8 years ago
Attachment #479689 - Flags: ui-review? → ui-review?(aza)

Comment 6

8 years ago
Created attachment 479693 [details]
Failed Application Of v1 patch

Hey Sean,

I can't get this to apply cleanly. Can you make up another one while being sure you are up-to-date?
(Reporter)

Comment 7

8 years ago
Comment on attachment 479689 [details] [diff] [review]
v1

Wow, that's a subtle bug! Looks like what's going on is the close boxes are removed because the thumbnails get small enough as you're resizing on the way to getting them stacked, and they don't get re-added when the thumbnail gets large enough again inside the stack, due to the fact that the close-size logic is inside the inStack else.

As for the fix, I don't think we want to force the close boxes on always; we just want them to obey the same rules as when they're not in a stack (Aza can confirm). The fix can be just to move the close logic outside of the "inStack" if/else. 

-      if (css.width) {

I would rather not get rid of this check; this makes sure we only adjust things that are size-dependent if the size changes, which makes setBounds more efficient for dragging, where the size doesn't change. In particular, we shouldn't be calling TabItems.update(this.tab) every time the thumbnail moves without resizing, as it causes a canvas repaint.
Attachment #479689 - Flags: feedback?(ian) → feedback-

Updated

8 years ago
Attachment #479689 - Flags: ui-review?(aza) → ui-review-
Status: NEW → ASSIGNED
(Assignee)

Comment 8

8 years ago
Created attachment 480660 [details] [diff] [review]
v2

This has Ian's suggestion for a simpler behavior.
Attachment #479689 - Attachment is obsolete: true
Attachment #480660 - Flags: ui-review?(aza)
Attachment #480660 - Flags: feedback?(ian)
(Reporter)

Updated

8 years ago
Attachment #480660 - Attachment is patch: true
Attachment #480660 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 9

8 years ago
Comment on attachment 480660 [details] [diff] [review]
v2

Lovely.
Attachment #480660 - Flags: feedback?(ian) → feedback+

Updated

8 years ago
Attachment #480660 - Flags: ui-review?(aza) → ui-review+
(Assignee)

Updated

8 years ago
Attachment #480660 - Flags: review?(dolske)
Attachment #480660 - Flags: review?(dolske) → review+
(Assignee)

Comment 10

8 years ago
Created attachment 483314 [details] [diff] [review]
v3 (changed checkin message)

ready for checkin
Attachment #480660 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
(Reporter)

Comment 11

8 years ago
http://hg.mozilla.org/mozilla-central/rev/e2dde7f7abed
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
verified with nightly minefield builds of 20101021

stacked has close button while at larger sizes of stacked, but disappears as stack icon gets smaller.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.