Closed Bug 1659079 Opened 2 months ago Closed 1 month ago

Make the Calendar Summary dialog themeable

Categories

(Calendar :: General, task)

Tracking

(thunderbird_esr78+ fixed, thunderbird80 fixed)

VERIFIED FIXED
81 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird80 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(2 files, 1 obsolete file)

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

Really simply doable with two additional lines.

Assignee: nobody → richard.marti
Attachment #9170006 - Flags: review?(paul)

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.

Attachment #9170006 - Attachment is obsolete: true
Attachment #9170006 - Flags: review?(paul)

Now with working popup.

Attachment #9170036 - Flags: review?(paul)

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

Flags: needinfo?(richard.marti)
Summary: Make the Summary dialog themeable → Make the Calendar Summary dialog themeable

This is my plan.

Flags: needinfo?(richard.marti)
Comment on attachment 9170036 [details] [diff] [review]
1659079-summary-dialog-themeable.patch

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

Changes look good.
Attachment #9170036 - Flags: review?(paul) → review+
Target Milestone: --- → 81 Branch

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.

Attachment #9170245 - Flags: approval-calendar-esr?(paul)
Attachment #9170245 - Flags: approval-calendar-beta?(paul)
Depends on: 1575195, 1647855

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

(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 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.
Attachment #9170245 - Flags: approval-calendar-esr?(paul)
Attachment #9170245 - Flags: approval-calendar-esr+
Attachment #9170245 - Flags: approval-calendar-beta?(paul)
Attachment #9170245 - Flags: approval-calendar-beta+

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/894b78f5b8f2
Make the calendar summary dialog themeable. r=pmorris

Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED

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

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?

Sorry, never mind - it was bug 1659598

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

Status: RESOLVED → VERIFIED

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

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

Attachment #9170245 - Flags: approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.