Last Comment Bug 653146 - switch awesomescreen tabs immediately on tap, then load content
: switch awesomescreen tabs immediately on tap, then load content
Status: VERIFIED FIXED
: ux-userfeedback
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
:
Mentors:
Depends on:
Blocks: 640971
  Show dependency treegraph
 
Reported: 2011-04-27 09:13 PDT by Madhava Enros [:madhava]
Modified: 2011-07-14 21:34 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (4.66 KB, patch)
2011-05-13 07:38 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
no flags Details | Diff | Splinter Review
Patch v0.2 (4.81 KB, patch)
2011-05-13 08:03 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
wjohnston2000: review+
Details | Diff | Splinter Review

Description Madhava Enros [:madhava] 2011-04-27 09:13:03 PDT
Right now, when you tap on some awesomescreen tabs (i.e. remote tabs or history), sometimes nothing happens immediately. The browser eventually brings up the selected tab, it seems, when it has loaded the contents.  Instead, we should switch tabs immediately, and then show some progress indication, if necessary.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-04-27 09:22:24 PDT
Lets see if we can make some impact on this for FF6
Comment 2 Madhava Enros [:madhava] 2011-05-04 11:33:19 PDT
Design-wise, let's visually switch to the tab and have our throbber displayed in the empty list area below.  Maybe centered, vertically and horizontally, in the area until we have anything to show?

Actually, when items are shown, how do they appear? One by one? All at once?
Comment 3 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-13 07:38:54 PDT
Created attachment 532228 [details] [diff] [review]
Patch
Comment 4 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-13 07:40:17 PDT
The patch will probably resolved bug 653146 too
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-13 07:49:41 PDT
Comment on attachment 532228 [details] [diff] [review]
Patch

>+/* awesomescreen panels ---------------------------------------------------- */
>+historylist > hbox,
>+remotetabslist > hbox {
>+  display: none;
>+}
>+
>+historylist[loading="true"] > hbox,
>+remotetabslist[loading="true"] > hbox {
>+  background-color: white;
>+  display: -moz-box;
>+}
>+
>+historylist[loading="true"] > richlistbox,
>+remotetabslist[loading="true"] > richlistbox {
>+  visibility: collapse;
>+}

Add a class to the XBL (hbox and richlistbox) so you can be more specific. The richlistboxes already have a class, so use it.

"history-throbber-box" and "remotetabs-throbber-box" ?
Comment 6 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-13 08:03:44 PDT
Created attachment 532236 [details] [diff] [review]
Patch v0.2

done!
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-13 08:06:15 PDT
Comment on attachment 532236 [details] [diff] [review]
Patch v0.2

passing to Wes - I just did a drive by :)
Comment 8 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-13 08:09:28 PDT
(In reply to comment #7)
> Comment on attachment 532236 [details] [diff] [review] [review]
> Patch v0.2
> 
> passing to Wes - I just did a drive by :)

oups that was my initial intention but my fingers has just type your name without my consent!
Comment 9 Wesley Johnston (:wesj) 2011-05-16 10:15:27 PDT
Comment on attachment 532236 [details] [diff] [review]
Patch v0.2

>+      <xul:hbox class="history-throbber-box" flex="1" align="center" pack="center">
>+        <xul:image src="chrome://browser/skin/images/throbber.png" />
>+      </xul:hbox>

Do we need the image element, or can we just use background-image? If you leave the image in, source should be set in css using "list-style-image" on the box. Same in the other place

>+            let children = self._children;
>+            while (children.lastChild)
>+              children.removeChild(children.lastChild);

Seems cleaner to me to do it outside the setTimeout and not worry about hiding these in CSS, but I guess this should do the same... except faster? If so, neat!

>+            self.setAttribute("loading", "false");

Just remove the attribute.
Comment 10 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2011-05-17 03:00:03 PDT
http://hg.mozilla.org/mozilla-central/rev/3da7cbddd83e
Comment 11 Andreea Pod 2011-05-20 07:56:17 PDT
Verified fixed on build: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110520 Firefox/6.0a1 Fennec/6.0a1 
Device: LG Optimus 2X (Android 2.2)

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