When opening a new tab, scroll tab strip to show the new tab

RESOLVED WORKSFORME

Status

Firefox for Android Graveyard
General
RESOLVED WORKSFORME
7 years ago
5 years ago

People

(Reporter: wesj, Unassigned)

Tracking

({helpwanted})

Trunk
helpwanted

Details

(Whiteboard: [mentor=wjohnston@mozilla.com][good first bug])

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
When the user opens a new tab, on tablets we should scroll the tab strip (animated?) to show the new tab.

Updated

7 years ago
Keywords: helpwanted
Whiteboard: [mentor=wjohnston@mozilla.com][good first bug]

Comment 1

7 years ago
Hi. I would like to work on this... could you help me to get started?
The AddTab function is here:
http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser.js#727
I guess on setting the selected tab, the tab should scroll into view:
http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser.js#813
Under vbox id="tabs", there is a xul:scrollbox class=tabs-scrollbox
You can perhaps its  boxObject.ensureElementIsVisible() method and use the selected tab element as its argument.

I think that Firefox desktop tabbrowser binding is using something like that.
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3359

If you want to do it animated, you need to do something more, but I think it's better to first fix it non-animated and handle the animated part in a new bug.

Comment 3

7 years ago
Ok :) Im currently trying to build the fennec in my laptop. I will message as soon as i get it working. :) Do you use windows as development environment?
Yes. It's better to ask these questions on irc, btw: https://wiki.mozilla.org/IRC#Mozilla.27s_Chat_Server
Mobile developers can be found on the #mobile channel.

Comment 6

7 years ago
okay cool :)

Comment 7

7 years ago
hello, i have everything needed to get started. I was looking through some pages in mdn website, I saw Scrolling a child element into view  in https://developer.mozilla.org/en/XUL/scrollbox Is it how it has to be done?

Comment 8

7 years ago
fixed it! :)

Comment 9

7 years ago
Created attachment 566146 [details] [diff] [review]
Patch to enable auto scroll to tabstrip to show the new tab.

well, i guess this will fix the issue :) no animation..
Attachment #566146 - Flags: review?(wjohnston)
(Reporter)

Comment 10

6 years ago
Comment on attachment 566146 [details] [diff] [review]
Patch to enable auto scroll to tabstrip to show the new tab.

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

Looks good! Sorry, I didn't realize I was a mentor on this bug even, but looks like martjin helped anyway!

::: mobile/chrome/content/tabs.xml
@@ +254,5 @@
>              this.children.appendChild(tab);
>              this._updateWidth();
> +			let tabListScroller = this.boxObject.QueryInterface(Ci.nsIScrollBoxObject);
> +			tabListScroller.ensureElementIsVisible(tab);
> +            return tab;			

We use spaces instead of tabs, so we'll need to fix that before checkin. There's also some extra whitespace after this return tab call that can be removed.
Attachment #566146 - Flags: review?(wjohnston) → review+
Let's make sure this doesn't cause problems on the phone UI. If it does, we'll need to do a tablet UI check.

Comment 12

6 years ago
Created attachment 567015 [details] [diff] [review]
removed extra tabs and spaces.. please check :)

removed extra tabs and spaces (i guess i dint miss any :P ).. please check :)

Updated

6 years ago
Attachment #567015 - Flags: review?(wjohnston)

Comment 13

6 years ago
i noticed something, if the tab is half visible in the strip, then even if i click on the tab its not scrolling up to show the entire tab. let me try to fix that as well..

Comment 14

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #11)
> Let's make sure this doesn't cause problems on the phone UI. If it does,
> we'll need to do a tablet UI check.

I think something is wrong, but as you know the desktop version has some problems right? So is it possible to check it from your side? I dragged page to right in phone UI to show the tab strip, which completely messed up every tabs.
Wes, is this still a valid bug for a new contributor to be working on? In other words, does this still apply to the Native UI, so that the work done won't be wasted?
Does this still apply?
(Reporter)

Comment 17

6 years ago
No. I'm going to close this bug. This should be fixed in the new native UI.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 18

5 years ago
Comment on attachment 567015 [details] [diff] [review]
removed extra tabs and spaces.. please check :)

Clearing this request since this is resolved.
Attachment #567015 - Flags: review?(wjohnston)
You need to log in before you can comment on or make changes to this bug.