Closed Bug 685692 Opened 8 years ago Closed 8 years ago

"Move to Group" should always insert the moved tab into the same place

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 9

People

(Reporter: ttaubert, Assigned: raymondlee)

Details

Attachments

(1 file, 1 obsolete file)

When using the tab context menu item "Move to Group" we should insert the moved tab always into the same place. I'd say we append it but the UX team might oppose that. Asking the UX team where to insert those tabs.
Assignee: nobody → raymond
At the moment, it moves tab to the first position of target group.  WIll make it add to the group as last element.
Attached patch v1 (obsolete) — Splinter Review
Attachment #561153 - Flags: review?(dietrich)
Comment on attachment 561153 [details] [diff] [review]
v1

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

::: browser/base/content/tabview/groupitems.js
@@ +2604,5 @@
>      // add tab item to a groupItem
>      if (groupItemId) {
>        groupItem = GroupItems.groupItem(groupItemId);
>        groupItem.add(tab._tabViewTabItem);
> +      groupItem.reorderTabsBasedOnTabItemOrder()

looks fine, r=me. and btw, i totally dig how readable panorama tests are.

one question though: should these two methods result in a different order? what's the use-case for a reordering that puts an appended tab at the front of a group?
Attachment #561153 - Flags: review?(dietrich) → review+
(In reply to Dietrich Ayala (:dietrich) from comment #3)
> one question though: should these two methods result in a different order?
> -      UI.setReorderTabItemsOnShow(groupItem);
This method is for reordering tab items in Panorama based on the tab order on the tab bar.  The issue of this bug is that the tabs on tab bar is not reordered before entering Panorama, therefore, the moved tab appears in the new group in Panorama in the order of tabs on tabbar.

> +      groupItem.reorderTabsBasedOnTabItemOrder()
To solve this, we use the above method to keep the order of tabs on tab bar and tab items in Panorama in sync.  When a tab is moved to another group  using "Move to Group" menu item, the moved tab item in Panorama would be appended to the new group list. we call the above method so when user enters next time, the moved tab would appear at the end of list.


> what's the use-case for a reordering that puts an appended tab at the front
> of a group?
For example, 

In Panorama
Tab 1, Tab 2 in Group A
Tab 3, Tab 4 in Group B

Group A is being displayed on tab bar.  Here is the tab order on tab bar
Tab 1, Tab 2, Tab 3 (hidden), Tab 4 (hidden)

Move Tab 1 to Group B and then enter Panorama. The items in Group B is reordered based on tab order on tab bar.  Tab 1 appears as the first item in Group B 
Tab 1 in Group A
Tab 1, Tab 3, Tab 4 in Group B

The patch solves this problem.
Attachment #561153 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a9edf67c8954
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 9
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111026 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111026 Firefox/10.0a1

When using move to group from the tab context menu, the moved tab is now always inserted in the last position. (as specified in comment 2). Verified on Windows XP, 7, Ubuntu 11.10, Mac OS 10.6 on Firefox 9 and 10 (Aurora and Nightly)
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.