836867 - The Find Toolbar should transition when opening and closing

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 1

[Dão Gottwald [: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.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v1.1

Comment 3

[u428464]

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).

Comment 4

[Alfred Kayser]

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}

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

This approach is simpler and should work without causing issues with 3rd-party themes.

Comment 6

[Dão Gottwald [: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

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Thanks for the review. I removed the default states and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/299893af8938

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/299893af8938