The Find Toolbar should transition when opening and closing

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

(Depends on 1 bug)

Trunk
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch Patch (obsolete) — Splinter Review
No description provided.
Attachment #708713 - Flags: review?(dao)
Comment on attachment 708713 [details] [diff] [review]
Patch

>+      <property name="hidden">
>+        <getter>
>+          return !this.classList.contains("slideIn") ||
>+                 this.hasAttribute("hidden");
>+        </getter>
>+        <setter><![CDATA[
>+          if (val) {
>+            this.addEventListener("transitionend", function onTransitionEnd() {
>+              this.removeEventListener("transitionend", onTransitionEnd);
>+              this.setAttribute("hidden", "true");
>+            })
>+            this.classList.remove("slideIn");

We'd infinitely wait for the transitionend event here when a theme doesn't add a transition. We could set a stub transition in xul.css to handle that.
Attachment #708713 - Flags: review?(dao) → review-
Posted patch Patch v1.1 (obsolete) — Splinter Review
Attachment #708713 - Attachment is obsolete: true
Attachment #708747 - Flags: review?(dao)
Blocks: 750212
This transition will make the bar appear from the bottom on opening and the other way on closing ?

If so the transition will have to be changed when the find bar will be resdesigned (bug 776708).
Can the .slideIn be changed to .slideOut?

The current patch doesn't allow backwards compatibility for themes, as the
margin-bottom:-1em and opacity:0 cannot be set in FF's before this patch.

By using a slideOut to specify the moving direction one can leave the normal
style rule for findbar as-is:
findbar.slideOut{
	margin-bottom:-1em;
	opacity:0}
Posted patch Patch v2Splinter Review
This approach is simpler and should work without causing issues with 3rd-party themes.
Attachment #708747 - Attachment is obsolete: true
Attachment #708747 - Flags: review?(dao)
Attachment #710764 - Flags: review?(dao)
Comment on attachment 710764 [details] [diff] [review]
Patch v2

> findbar {
>   border-top: 2px solid;
>   -moz-border-top-colors: ThreeDShadow ThreeDHighlight;
>   padding-bottom: 1px;
>   min-width: 1px;
>+  transition-property: margin-bottom, opacity, visibility;
>+  transition-duration: 150ms, 150ms, 0s;
>+  transition-timing-function: ease-in-out, ease-in-out, linear;
>+  margin-bottom: 0;
>+  opacity: 1;
>+}

margin-bottom:0 and opacity:1 are the default states and redundant. r=me with them removed
Attachment #710764 - Flags: review?(dao) → review+
Thanks for the review. I removed the default states and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/299893af8938
https://hg.mozilla.org/mozilla-central/rev/299893af8938
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
No longer blocks: 750212
Depends on: 1327839
You need to log in before you can comment on or make changes to this bug.