Closed Bug 852622 Opened 7 years ago Closed 7 years ago

Need new ring throbber for progress indicator

Categories

(Firefox for Metro Graveyard :: Theme, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: rsilveira, Assigned: jimm)

References

Details

Attachments

(2 files, 1 obsolete file)

No longer blocks: metrov1triage
Blocks: 868094
No longer blocks: Backlog-MetroDesign
Assignee: nobody → jmathies
Attached patch throbber patch v.1 (obsolete) — Splinter Review
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!
Attached file throbber html
I've got this working now by including the cssthrobber.css in browser.xul vs. the binding.
Attachment #745546 - Attachment is obsolete: true
Attachment #745654 - Flags: review?(fyan)
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
Yeah right now just in settings. I don't think we would use something like this with content display.
(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 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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Blocks: 869805
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.