Closed Bug 1636238 Opened 5 years ago Closed 5 years ago

Convert remaining PNG icons to SVG for visual consistency

Categories

(Thunderbird :: Theme, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 78.0

People

(Reporter: aleca, Assigned: Paenglab)

Details

Attachments

(7 files, 2 obsolete files)

Attached image icons.png

We're using some PNGs around the app which should be converted to SVGs and complete the visual refresh to better support HiDPI monitors and improve UI consistency.

After a quick investigation, these are the sections that present replaceable icons:

  • Account settings richilist for IM accounts.
  • Chat contacts and availability indicator.
  • Compose Address Book menu item.
  • Missing primary icon in the Get Messages dropdown menulist.
  • Online/Offline indicator in the status bar.

Attachment for reference.

Magnus, FX removed all icons from the menus on Linux, Mac and Windows have none.

What do you think about removing them on TB too? Then we are in synch with Mac and Windows. The issue is also that we have no HiDPI icons, like the AB icon in composer menu, that look good on HiDPI Linux desktops.

Using SVG icons is also no option as they don't fit with the native icons we also use.

Flags: needinfo?(mkmelin+mozilla)

It's a bit of a shame, but gtk stock icons are deprecated, so I guess it's time to let go.

Flags: needinfo?(mkmelin+mozilla)
Attached patch 1636238-apply-more-SVG.patch (obsolete) — Splinter Review

This patch implements the things you wrote in comment 0 except the IM parts. They should be done in bug 827092.

I also removed a lot of not used icons. In total 120 icons removed.

Attachment #9147349 - Flags: review?(alessandro)

Forgot to mention: this patch removes the menu icons on Linux. Mac and Windows have done this already long time ago.

Massive patch!
Thank you so much for taking care of this. I'll take some time to review and be sure to check every single section and line of code.
Apologies for the wait.

Comment on attachment 9147349 [details] [diff] [review] 1636238-apply-more-SVG.patch Review of attachment 9147349 [details] [diff] [review]: ----------------------------------------------------------------- After a first round of review this looks pretty good, thank you so much. There are a couple of nits that need fixing, but I'm gonna write them alongside the screenshots I took.
Attachment #9147349 - Flags: review?(alessandro) → feedback+

The zoom of the Gloda view timeline is in a very strange position.
Would be possible to move it to the right of the "Toggle timeline" button?
Also, That dark background color on the zoom button is too dark. We could have that zoom icon without a bg-color in its natural state, and using the primary color on hover.

The icon used for the deleted attachment is a bit too busy.
We already have the file, which acts as a container, so we shouldn't use the cancel icon which has a circle background (another container).
A simple red X icon, or a red bar like you did for the offline icon might work better.

Attached patch 1636238-apply-more-SVG.patch (obsolete) — Splinter Review

(In reply to Alessandro Castellani (:aleca) from comment #7)

Created attachment 9147465 [details]
Screenshot from 2020-05-11 10-55-29.png

The zoom of the Gloda view timeline is in a very strange position.
Would be possible to move it to the right of the "Toggle timeline" button?
Also, That dark background color on the zoom button is too dark. We could have that zoom icon without a bg-color in its natural state, and using the primary color on hover.

In non-hovered state there is now no background. I changed the hover colours to use system colours, Highlight and HighlightText. I also added a border with the gloda background colour to separate the button when it overlaps a rectangle (hoe is this named?).

I don't move the button because this is too much for this bug because this button is created in JS. This should be done in a separate bug.

(In reply to Alessandro Castellani (:aleca) from comment #8)

Created attachment 9147466 [details]
Screenshot from 2020-05-11 10-57-23.png

The icon used for the deleted attachment is a bit too busy.
We already have the file, which acts as a container, so we shouldn't use the cancel icon which has a circle background (another container).
A simple red X icon, or a red bar like you did for the offline icon might work better.

I changed it to a red X.

Attachment #9147349 - Attachment is obsolete: true
Attachment #9147607 - Flags: review?(alessandro)
Comment on attachment 9147607 [details] [diff] [review] 1636238-apply-more-SVG.patch Review of attachment 9147607 [details] [diff] [review]: ----------------------------------------------------------------- This is great, thanks. I finally figured why the get messages icon was bothering me. The rectangle has 1px curvature but all our other icons that come with a rectangle (email, folder, etc) have a 2px radius. I'm gonna attach the updated SVGs for `getmsg` and `get-all` which will respect the curvature we're using everywhere else. The `get-all` basically uses the `share.svg` Photon icon with a flipped arrow. Another little update is the Phishing icon, I like the shield, but due to the severity of the warning, we should use an exclamation mark instead of the info mark inside the shield. Attaching those files so you can use them (and probably optimize them). After that, this is an r+ and ready to go. I guess we should launch a try run to be sure no test is affected by these UI changes. It shouldn't, but we can never be too sure.
Attachment #9147607 - Flags: review?(alessandro) → review+
Attached image get-all.svg
Attached image getmsg.svg
Attached image phishing.svg

Thanks. Yes, looks better with this icons.
Try https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=05a8e5c368b7fb720aea4e5b7bc2543a4fa8e1ea shows no relevant failures.

Attachment #9147607 - Attachment is obsolete: true
Attachment #9147915 - Flags: review+
Target Milestone: --- → Thunderbird 78.0
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: