Last Comment Bug 625955 - Group's columns value is not reset correctly after tabs are removed.
: Group's columns value is not reset correctly after tabs are removed.
Status: RESOLVED FIXED
[visual][good first bug]
: polish
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P4 normal
: Firefox 10
Assigned To: Luyun Xie[:luyunxie]
:
Mentors:
: 628158 630171 666213 672794 689279 (view as bug list)
Depends on: 685456
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-14 16:17 PST by Michael Yoshitaka Erlewine [:mitcho]
Modified: 2016-04-12 14:00 PDT (History)
12 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Video (319.82 KB, video/ogg)
2011-01-14 16:17 PST, Michael Yoshitaka Erlewine [:mitcho]
no flags Details
update column number even if there is only one tab in the group (603 bytes, patch)
2011-08-31 20:03 PDT, Luyun Xie[:luyunxie]
ttaubert: review-
Details | Diff | Review
patch with testcase (2.13 KB, patch)
2011-09-27 19:33 PDT, Luyun Xie[:luyunxie]
ttaubert: review+
Details | Diff | Review
fresh patch (2.42 KB, patch)
2011-09-28 20:20 PDT, Luyun Xie[:luyunxie]
ttaubert: checkin+
Details | Diff | Review

Description Michael Yoshitaka Erlewine [:mitcho] 2011-01-14 16:17:28 PST
Created attachment 504027 [details]
Video

When the second to last tab is removed from a GroupItem, the Group Item continues to be rendered as if there are two tabs and therefore must have two columns.

While mostly visual in nature, it seems like this may be one part of bug 622872 (blocker).
Comment 1 Ian Gilman [:iangilman] 2011-01-19 11:30:37 PST
I believe that the original design was that above a certain width, a lone tab would take only half the available width of the group. Otherwise you end up with a huge tab that fills the group entirely. By providing that extra space, we invite more tabs to be dropped in, and we avoid having the group just be an extra frame around the tab. Of course below a certain width, it made sense for the tab to fill the group, other wise it would be too small.

It appears that the code that did that has eroded such that it kind of does it sometimes. I would propose we use this bug to reinstate the original design.

Also note that when you first go into Panorama, if there's a group with a single tab, that tab fills the group at first, but then stops doing so when you start messing with it. That should be fixed as well (so it starts out half-width if need be).
Comment 2 Tim Taubert [:ttaubert] 2011-01-31 05:22:26 PST
*** Bug 630171 has been marked as a duplicate of this bug. ***
Comment 3 Tim Taubert [:ttaubert] 2011-06-22 03:48:41 PDT
*** Bug 666213 has been marked as a duplicate of this bug. ***
Comment 4 Tim Taubert [:ttaubert] 2011-07-21 06:47:14 PDT
*** Bug 672794 has been marked as a duplicate of this bug. ***
Comment 5 Luyun Xie[:luyunxie] 2011-08-31 19:57:05 PDT
(In reply to Ian Gilman [:iangilman] from comment #1)
> I believe that the original design was that above a certain width, a lone
> tab would take only half the available width of the group. Otherwise you end
> up with a huge tab that fills the group entirely. By providing that extra
> space, we invite more tabs to be dropped in, and we avoid having the group
> just be an extra frame around the tab. Of course below a certain width, it
> made sense for the tab to fill the group, other wise it would be too small.
>
Not exactly. The original design is to make tabs flexible and fills the group space as much as possible. Whenever you drag a tab into a group, before you drop that tab, tabs inside the targeting group will automatically rearrange to make space for the new comer. I think this "half width tab" is just a kind mistake.
Comment 6 Luyun Xie[:luyunxie] 2011-08-31 20:03:09 PDT
Created attachment 557397 [details] [diff] [review]
update column number even if there is only one tab in the group

This root cause is that the group is reluctant to update column information when there is only one tab in the group. Thus, when a tab is removed from a two-columns-two-tabs group, the column number remains 2.
Comment 7 Raymond Lee [:raymondlee] 2011-09-01 20:31:19 PDT
Comment on attachment 557397 [details] [diff] [review]
update column number even if there is only one tab in the group

Request a review
Comment 8 Raymond Lee [:raymondlee] 2011-09-01 21:57:40 PDT
*** Bug 628158 has been marked as a duplicate of this bug. ***
Comment 9 Tim Taubert [:ttaubert] 2011-09-05 10:48:24 PDT
Comment on attachment 557397 [details] [diff] [review]
update column number even if there is only one tab in the group

Review of attachment 557397 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, that looks good to me! Let's add a test for this. r- only for the missing test.
Comment 10 Tim Taubert [:ttaubert] 2011-09-26 19:13:44 PDT
*** Bug 689279 has been marked as a duplicate of this bug. ***
Comment 11 Luyun Xie[:luyunxie] 2011-09-27 19:33:46 PDT
Created attachment 562951 [details] [diff] [review]
patch with testcase

Thanks for advice from Tim.
Comment 12 Tim Taubert [:ttaubert] 2011-09-28 05:18:22 PDT
Comment on attachment 562951 [details] [diff] [review]
patch with testcase

Review of attachment 562951 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks! Can you please prepare the patch for checkin?

Here's some advice on how to do that: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Comment 13 Luyun Xie[:luyunxie] 2011-09-28 20:20:54 PDT
Created attachment 563284 [details] [diff] [review]
fresh patch

r=ttaubert
Comment 14 Tim Taubert [:ttaubert] 2011-09-29 16:09:52 PDT
Thanks for the patch!

https://hg.mozilla.org/integration/fx-team/rev/268ba6fecae0
Comment 15 Rob Campbell [:rc] (:robcee) 2011-10-04 05:19:15 PDT
https://hg.mozilla.org/mozilla-central/rev/268ba6fecae0

Note You need to log in before you can comment on or make changes to this bug.