Closed Bug 1160822 Opened 9 years ago Closed 9 years ago

Zoom button gets cut off after hiding/unhiding search results visualisation


(Thunderbird :: Search, defect)

Not set


(Not tracked)

Thunderbird 41.0


(Reporter: aleth, Assigned: aleth)




(1 file, 2 obsolete files)

In the faceted search results, click the bar graph icon to hide and then unhide the date visualisation of the search results.
The height of the visualisation doesn't seem to get restored correctly: the zoom button to zoom in/out of the date visualisation is cut off at the top.
Attached patch timelinerestart.diff (obsolete) — Splinter Review
There were actually a bunch of related bugs here:

- If the timeline was hidden, after a restart it would not open properly if one clicked the button, as the style attribute was never removed.

- The zoom button you get on clicking on one of the timeline bars was cut off after showing/hiding the timeline as showing/hiding changed overflow:visible to overflow:hidden.

- When hiding the timeline, the timeline wasn't actually being completely hidden due to the presence of padding, so you could still see the top of one of the bars in the bar graph after hiding the timeline.

This patch works, but I can't help thinking there should be an easier way to write the CSS. I don't know animations/transitions well enough to see how though. (Note in particular "animating" the overflow property doesn't seem to work.)
Assignee: nobody → aleth
Attachment #8600683 - Flags: review?(josiah)
Blocks: 1014609
Comment on attachment 8600683 [details] [diff] [review]

Review of attachment 8600683 [details] [diff] [review]:

I haven't tested this, but it seems like you want transition here, not animation. You can just give #facet-date a `transition: all .3s ease-in;` rule and remove all the animation stuff.

::: mail/base/content/glodaFacetView.js
@@ +601,5 @@
> + = "none";
> +    else
> +      facetDate.removeAttribute("style"); // removes style.overflow
> +    facetDate.classList.remove("slideDown");
> +    facetDate.classList.add("slideUp");

Don't bother adding classes, just use attributes. That makes the CSS transitions easier as well.
Attachment #8600683 - Flags: review?(josiah)
Seems like you really just need two things: A default facet-date, and a hidden one. Then use the transition to switch between them.

#facet-date {
   transition: all .3s ease-in;
   /* Default properties here. /*
#facet-date[hide="true"] {
   /* Hidden properties */
Attached patch timelinerestart.diff v2 (obsolete) — Splinter Review
Thanks, using a transition indeed simplified things a lot.
Attachment #8600683 - Attachment is obsolete: true
Attachment #8616283 - Flags: review?(josiah)
Comment on attachment 8616283 [details] [diff] [review]
timelinerestart.diff v2

Review of attachment 8616283 [details] [diff] [review]:

Just a couple CSS nits, but other than that, r+. Thanks!

::: mail/base/content/glodaFacetView.css
@@ +813,5 @@
>    color: #729fcf;
>  }
> +#facet-date {
> +  transition: all .3s ease-in;

transition rules generally are the last rule of a selector, so could you move this down? I realize this is how I demonstrated it in my example, but I was wrong there. :)

@@ +821,5 @@
>    overflow: hidden;
>  }
> +#facet-date[hide="true"] {
> +  transition: all .3s ease-in;

You don't need to re-declare this actually, the #facet-date rule will still apply here.
Attachment #8616283 - Flags: review?(josiah) → review+
> You don't need to re-declare this actually, the #facet-date rule will still apply here.

Good catch, thanks! I'd just copypasted the desired state.
Attachment #8616283 - Attachment is obsolete: true
Attachment #8616480 - Flags: review+
Keywords: checkin-needed -> FIXED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
Depends on: 1190092
You need to log in before you can comment on or make changes to this bug.