Closed Bug 597498 Opened 14 years ago Closed 14 years ago

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

Categories

(Firefox Graveyard :: Panorama, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 4.0

People

(Reporter: iangilman, Assigned: seanedunn)

References

Details

Attachments

(2 files, 2 obsolete files)

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
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?
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?
Blocks: 597763
Priority: P3 → P2
Target Milestone: --- → Firefox 4.0
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
Attached patch v1 (obsolete) — Splinter Review
Close button stays 100% opaque with no fade for stacked tabitems.
Attachment #479689 - Flags: ui-review?
Attachment #479689 - Flags: feedback?(ian)
Attachment #479689 - Flags: ui-review? → ui-review?(aza)
Hey Sean,

I can't get this to apply cleanly. Can you make up another one while being sure you are up-to-date?
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-
Attachment #479689 - Flags: ui-review?(aza) → ui-review-
Status: NEW → ASSIGNED
Attached patch v2 (obsolete) — Splinter Review
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)
Attachment #480660 - Attachment is patch: true
Attachment #480660 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 480660 [details] [diff] [review]
v2

Lovely.
Attachment #480660 - Flags: feedback?(ian) → feedback+
Attachment #480660 - Flags: ui-review?(aza) → ui-review+
Attachment #480660 - Flags: review?(dolske)
Attachment #480660 - Flags: review?(dolske) → review+
ready for checkin
Attachment #480660 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e2dde7f7abed
Status: ASSIGNED → RESOLVED
Closed: 14 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.

Attachment

General

Creator:
Created:
Updated:
Size: