New icon for "Save" (or "Save and Close") toolbar button

RESOLVED FIXED in 5.4

Status

Calendar
Dialogs
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: pmorris, Assigned: Paenglab)

Tracking

(Blocks: 1 bug)

Lightning 5.4
Dependency tree / graph

Details

Attachments

(4 attachments)

(Reporter)

Description

2 years ago
Now that we have a "Save" toolbar button it currently uses the same icon as "Save and Close".  These icons should be different so a new icon for "Save" (or for "Save and Close") is needed.
(Reporter)

Updated

2 years ago
Blocks: 1272540, 1312113
Version: Lightning 5.5 → Lightning 5.4

Comment 1

a year ago
Probably we should use the existing icon for save only and find something different for save & close.

Paenglab, do you have an idea for a suitable icon? We will need this for the ESR.
Flags: needinfo?(richard.marti)
(Assignee)

Comment 2

a year ago
Created attachment 8822231 [details]
SaveAndClose.png

MakeMyDay, what do you think about this? This is a screenshot from Win7. Linux would look the same. For Win8/10 and macOS and the inverted icons I need to create small differing ones which I'll do when we are content with the icon.

The icon is a combination of the save disc and a "Okay" checkmark.
Flags: needinfo?(richard.marti)
Attachment #8822231 - Flags: feedback?(makemyday)

Comment 3

a year ago
Comment on attachment 8822231 [details]
SaveAndClose.png

Thanks for coming up with a proposal.

In general, this looks good and the combination of the save icon and the checkmark is the best to visualize the combinded save & close opertation. Maybe it's worth to try how it would look with the checkmark being slightly smaller than in the current proposal, but I'm also be fine with taking it as is.

Can you prepare a patch to provide the new icons for all platforms?
Attachment #8822231 - Flags: feedback?(makemyday) → feedback+
(Assignee)

Comment 4

a year ago
Created attachment 8822748 [details] [diff] [review]
saveCloseIcon.patch

Okay, how about this?
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8822748 - Flags: review?(makemyday)
(Assignee)

Comment 5

a year ago
Created attachment 8822833 [details] [diff] [review]
saveCloseIcon-XP.patch aurora only

Because TB 53 does no more support XP I made the first patch without XP changes. This patch is for the XP changes applied on top of the first patch to apply on aurora which is the last version supporting XP.

Philipp, I n-i you to ask, how are the plans for Lightning to support XP? Since TB 52 the binary parts are in TB/SM itself and technically all future Lightning versions could work with TB 52 too. Do you still plan to bind the Lightning versions with the TB versions? If yes, I could remove all XP code from Lightning.

And there is also no more need to create platform specific XPIs. One XPI supports now all platforms.
Flags: needinfo?(philipp)
Attachment #8822833 - Flags: review?(makemyday)
(Assignee)

Comment 6

a year ago
Created attachment 8822834 [details]
Screenshot of save-close button on XP
Lightning versions will always be bound to Thunderbird versions due to how the build process works, but it is true that now compatibility could be more broad. For the same reason there will still be per-platform xpis. So IIUC from a theme perspective the e.g. linux xpi would also work on mac and windows? If this is the case we'd just have to check for preprocessor defines and make sure dialog icons are included, then I could really reduce it to one xpi on AMO.

I may simplify things in the future, but tb52 will stay as it was so feel free to remove xp support.
Flags: needinfo?(philipp)
(Assignee)

Comment 8

a year ago
Theme wise it works already since Lightning 3.1 (bug 882761) to use the Linux xpi on macOS or Windows (I tested this some month ago with ical.js enabled). So one xpi for all platforms should work now.

Comment 9

a year ago
Comment on attachment 8822748 [details] [diff] [review]
saveCloseIcon.patch

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

Thanks, looks good.
Attachment #8822748 - Flags: review?(makemyday) → review+

Comment 10

a year ago
Comment on attachment 8822833 [details] [diff] [review]
saveCloseIcon-XP.patch aurora only

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

r+ for aurora only.
Attachment #8822833 - Flags: review?(makemyday) → review+
(Assignee)

Updated

a year ago
Attachment #8822833 - Attachment description: saveCloseIcon-XP.patch → saveCloseIcon-XP.patch aurora only
Attachment #8822833 - Flags: approval-calendar-aurora?(philipp)
(Assignee)

Updated

a year ago
Attachment #8822748 - Flags: approval-calendar-aurora?(philipp)
(Assignee)

Updated

a year ago
Keywords: checkin-needed
Attachment #8822748 - Flags: approval-calendar-aurora?(philipp) → approval-calendar-aurora+
Attachment #8822833 - Flags: approval-calendar-aurora?(philipp) → approval-calendar-aurora+
(Assignee)

Comment 11

a year ago
https://hg.mozilla.org/comm-central/rev/3c3539b7c5d8f3b35b699166a12064c467a3fe2b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → 5.5
You need to log in before you can comment on or make changes to this bug.