1659079 - Make the Calendar Summary dialog themeable

Status: VERIFIED FIXED

(Calendar :: General, task)

Description

[Richard Marti (:Paenglab)]

We should also make the summary dialog themeable. Now it is the default dialog that is shown when double clicking a event.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1659079-summary-dialog-themeable.patch

Really simply doable with two additional lines.

Comment 2

[Richard Marti (:Paenglab)]

Comment on attachment 9170006 [details] [diff] [review]
1659079-summary-dialog-themeable.patch

When the event is recurring, the popup in the button isn't themed.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1659079-summary-dialog-themeable.patch

Now with working popup.

Comment 4

[Wayne Mery (:wsmwk)]

Please request beta and esr uplift as soon as this lands.

Comment 5

[Richard Marti (:Paenglab)]

This is my plan.

Comment 6

[Paul Morris [:pmorris]]

Comment on attachment 9170036 [details] [diff] [review]
1659079-summary-dialog-themeable.patch

Review of attachment 9170036 [details] [diff] [review]:
-----------------------------------------------------------------

Changes look good.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: 1659079-summary-dialog-themeable-beta-ESR.patch

Are bug 1575195 and bug 1647855 planned to uplift to ESR78?

If yes, we can wait with this patch and land the other patch in this bug when the other two bugs are uplifted. If not, this patch applies to beta and ESR.

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9170245 [details] [diff] [review]
1659079-summary-dialog-themeable-beta-ESR.patch

FWIW I'm OK with taking this, with Paul's blessing

Comment 9

[Paul Morris [:pmorris]]

(In reply to Richard Marti (:Paenglab) from comment #7)

Are bug 1575195 and bug 1647855 planned to uplift to ESR78?

I'm not sure whether it makes sense to uplift them or not. If we do decide to uplift, we need to also do bug 1651779 and likely bug 1651783 as part of the uplift.

There are some other changes we'd like to make to this area (e.g. adding a "Delete" button to the read-only dialog, and see Alex's design at the top of bug 1575195). So that's the case for holding off on uplift, to get things to a more 'finished' state for the next ESR and avoid some UI/UX churn.

Comment 10

[Paul Morris [:pmorris]]

Comment on attachment 9170245 [details] [diff] [review]
1659079-summary-dialog-themeable-beta-ESR.patch

Review of attachment 9170245 [details] [diff] [review]:
-----------------------------------------------------------------

Approving uplift.  I think it makes sense to go ahead and uplift this patch.  Even if we decide to uplift bug 1575195 and bug 1647855 there are other bugs that need to be fixed and uplifted with them (see previous comment), so better not to block this change on that.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/894b78f5b8f2
Make the calendar summary dialog themeable. r=pmorris

Comment 12

[Magnus Melin [:mkmelin]]

Suspecting the bc1 failures are from this (comm/calendar/test/browser/preferences/browser_categoryColors.js)

Comment 13

[Richard Marti (:Paenglab)]

Hmm, I don't change any ID or class nor do I rearrange something. I only add some styling. How can this patch break such tests?

Comment 14

[Magnus Melin [:mkmelin]]

Sorry, never mind - it was bug 1659598

Comment 15

[Rob Lemley [:rjl]]

Thunderbird 80.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/b39b483c45d3

Comment 16

[WaltS48 [:walts48]]

Looks good in my test of the 80.0b5 release candidate on Ubuntu 18.04.5 LTS.

Comment 17

[Rob Lemley [:rjl]]

Comment on attachment 9170245 [details] [diff] [review]
1659079-summary-dialog-themeable-beta-ESR.patch

[Triage Comment]
This is meant for esr78, setting right flag.

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 78.2.0:
https://hg.mozilla.org/releases/comm-esr78/rev/5b23922c463e

Comment 19

[Lasana Murray]

Attachment: Bug 1659079 - Remove custom style from recurring event edit context menu. r=aleca,Paenglab