Need new ring throbber for progress indicator

RESOLVED FIXED in Firefox 23

Status

Firefox for Metro
Theme
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: rsilveira, Assigned: jimm)

Tracking

Trunk
Firefox 23
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Updated

5 years ago
Blocks: 841214
(Assignee)

Updated

5 years ago
Blocks: 838497

Updated

5 years ago
No longer blocks: 841214
(Assignee)

Updated

5 years ago
Blocks: 868094
No longer blocks: 838497
(Assignee)

Updated

5 years ago
Assignee: nobody → jmathies
(Assignee)

Comment 1

5 years ago
Created attachment 745546 [details] [diff] [review]
throbber patch v.1
(Assignee)

Comment 2

5 years ago
I have this binding working for the most part. You can add it to xul via:

<cssthrobber id="throbtest"></cssthrobber>

and you have to specify some css to define the dims of the elements as such:

#throbtest .progressContainer {
  width: 25px;
  height: 25px;
  border: 1px solid lightgray;
}

#throbtest .progressBall {
  margin: 2px;
  width: 22px;
  height: 22px;
  border: 1px solid lightgray;
}

#throbtest .progressBallInner {
  width: 5px;
  height: 5px;
  border-radius: 3px;
}

There's just one remaining problem, the animations don't seem to work. Not sure what's wrong, the binding just seems to ignore the animation: prop and the keyframe data. It works fine in html. If anyone has any ideas please post them!
(Assignee)

Comment 3

5 years ago
Created attachment 745549 [details]
throbber html
(Assignee)

Comment 4

5 years ago
I've got this working now by including the cssthrobber.css in browser.xul vs. the binding.
(Assignee)

Comment 5

5 years ago
Created attachment 745654 [details] [diff] [review]
throbber patch v.2
Attachment #745546 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #745654 - Flags: review?(fyan)

Comment 6

5 years ago
Where do we plan to use this throbber?

Bug 759252 was WONTFIX'd, because it turned out that using CSS animations had more overhead, resulting in worse performance than an animated PNG. The downside of the animated PNG is of course that you need to hardcode each frame. See the discussion starting at bug 759252 comment 13.

Also, using an animated PNG results in all the throbbers being in sync with each other, which is generally preferred if multiple throbbers are displayed simultaneously.

I too prefer using CSS animations in theory, but I just want to make sure that we don't accidentally regress the experience in execution.
(In reply to Frank Yan (:fryn) from comment #6)
> Where do we plan to use this throbber?
> 
> Bug 759252 was WONTFIX'd, because it turned out that using CSS animations
> had more overhead, resulting in worse performance than an animated PNG. The
> downside of the animated PNG is of course that you need to hardcode each
> frame. See the discussion starting at bug 759252 comment 13.
> 
> Also, using an animated PNG results in all the throbbers being in sync with
> each other, which is generally preferred if multiple throbbers are displayed
> simultaneously.
> 
> I too prefer using CSS animations in theory, but I just want to make sure
> that we don't accidentally regress the experience in execution.

Mostly used in showing progress for actions in the Setting and Sync.
Sync flyout: Syncing, Removing device, Adding device
Setting flyout: Clearing private data, importing data from other browsers
(Assignee)

Comment 8

5 years ago
Yeah right now just in settings. I don't think we would use something like this with content display.
(Assignee)

Comment 9

5 years ago
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #7)
> Setting flyout: Clearing private data, importing data from other browsers

We'll need a bug on this btw if we use this. I didn't update it in my patch work for sync, so it's still using the old fennec circle throbber.

Comment 10

5 years ago
Comment on attachment 745654 [details] [diff] [review]
throbber patch v.2

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

Looks good. :)
A few suggestions below, but they don't block landing this.

::: browser/metro/base/content/bindings/cssthrobber.xml
@@ +13,5 @@
> +    <content>
> +      <html:div class="progressContainer">
> +        <html:div class="progressBall progressBall_1">
> +          <html:div class="progressBallInner" />
> +        </html:div>

You could simplify the markup to:
<html:div class="progressBall progressBall_1"/>
and style in the inner element as .progressBall::after.

Another step further would be to simply have five:
<html:div class="progressBall"/>
and set the delays with .progressBall:nth-child(1), .progressBall:nth-child(2), etc.

::: browser/metro/theme/cssthrobber.css
@@ +95,5 @@
> +
> +  100% {
> +    opacity: 0;
> +    transform: rotate(900deg);
> +  }

You can combine these two:
76%, 100% { ... }
Attachment #745654 - Flags: review?(fyan) → review+
https://hg.mozilla.org/mozilla-central/rev/b86f728bb8ef
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23

Updated

5 years ago
Blocks: 869805

Updated

5 years ago
Blocks: 883415
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.