Closed Bug 1322261 Opened 8 years ago Closed 8 years ago

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


(Calendar :: Dialogs, defect)

Lightning 5.4
Not set


(Not tracked)



(Reporter: pmorris, Assigned: Paenglab)


(Blocks 1 open bug)



(4 files)

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.
Version: Lightning 5.5 → Lightning 5.4
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)
Attached 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 on attachment 8822231 [details]

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+
Attached patch saveCloseIcon.patch β€” β€” Splinter Review
Okay, how about this?
Assignee: nobody → richard.marti
Attachment #8822748 - Flags: review?(makemyday)
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)
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 on attachment 8822748 [details] [diff] [review]

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

Thanks, looks good.
Attachment #8822748 - Flags: review?(makemyday) → review+
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+
Attachment #8822833 - Attachment description: saveCloseIcon-XP.patch → saveCloseIcon-XP.patch aurora only
Attachment #8822833 - Flags: approval-calendar-aurora?(philipp)
Attachment #8822748 - Flags: approval-calendar-aurora?(philipp)
Keywords: checkin-needed
Attachment #8822748 - Flags: approval-calendar-aurora?(philipp) → approval-calendar-aurora+
Attachment #8822833 - Flags: approval-calendar-aurora?(philipp) → approval-calendar-aurora+
Closed: 8 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.