Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in Thunderbird 41.0

Status

Thunderbird
Search
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: aleth, Assigned: aleth)

Tracking

Trunk
Thunderbird 41.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
Created attachment 8600683 [details] [diff] [review]
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.)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attachment #8600683 - Flags: review?(josiah)
(Assignee)

Updated

2 years ago
Blocks: 1014609
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.
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 */
}
(Assignee)

Comment 4

2 years ago
Created attachment 8616283 [details] [diff] [review]
timelinerestart.diff v2

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+
(Assignee)

Comment 6

2 years ago
Created attachment 8616480 [details] [diff] [review]
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.
Attachment #8616283 - Attachment is obsolete: true
Attachment #8616480 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 7

2 years ago
https://hg.mozilla.org/comm-central/rev/2970e7c552aa -> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
(Assignee)

Updated

2 years ago
Depends on: 1190092
You need to log in before you can comment on or make changes to this bug.