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

RESOLVED FIXED in 5.4

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pmorris, Assigned: Paenglab)

Tracking

(Blocks 1 bug)

Lightning 5.4
Dependency tree / graph

Details

Attachments

(4 attachments)

Reporter

Description

3 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

3 years ago
Version: Lightning 5.5 → Lightning 5.4

Comment 1

3 years 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

3 years ago
Posted image 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

3 years 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

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

Comment 5

3 years ago
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)
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

3 years 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

3 years 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

3 years 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

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

Updated

3 years ago
Attachment #8822748 - Flags: approval-calendar-aurora?(philipp)
Assignee

Updated

3 years 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

3 years ago
https://hg.mozilla.org/comm-central/rev/3c3539b7c5d8f3b35b699166a12064c467a3fe2b
Status: ASSIGNED → RESOLVED
Closed: 3 years 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.