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

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[aleth [:aleth]]

STR:
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.

Comment 1

[aleth [:aleth]]

Attachment: timelinerestart.diff

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

Comment 2

[Josiah Bruner [:jsbruner]]

Comment on attachment 8600683 [details] [diff] [review]
timelinerestart.diff

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 @@
> +      facetDate.style.display = "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.

Comment 3

[Josiah Bruner [:jsbruner]]

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 */
}

Comment 4

[aleth [:aleth]]

Attachment: timelinerestart.diff v2

Thanks, using a transition indeed simplified things a lot.

Comment 5

[Josiah Bruner [:jsbruner]]

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.

Comment 6

[aleth [:aleth]]

Attachment: timelinerestart.diff v3

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

Comment 7

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/2970e7c552aa -> FIXED