Closed Bug 836867 Opened 9 years ago Closed 9 years ago

The Find Toolbar should transition when opening and closing

Categories

(Toolkit :: Find Toolbar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: jaws, Assigned: jaws)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Attached 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-
Attached 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}
Attached 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: 9 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.