Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add symbolic app icon #27035

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

jsparber
Copy link
Contributor

@jsparber jsparber commented Nov 2, 2023

GNOME Shell uses the symbolic icon in some places.

@ilya-fedin
Copy link
Contributor

Can you explain better why should this be merged? As-is it's not clear what this change would affect.

@jsparber
Copy link
Contributor Author

jsparber commented Nov 6, 2023

Can you explain better why should this be merged? As-is it's not clear what this change would affect.

This doesn't have any down sides, but when ever the theme (on GNOME but probably also on other platforms) uses a symbolic icon the newly added icon is used, in other places it will keep using the hicolor one.

GNOME uses the symbolic icon in the topbar next to the app menu and in notifications when the app doesn't provide a icon for the notification.

@ilya-fedin
Copy link
Contributor

So you're proposing a window icon change. I'm not sure that's allowed...
@john-preston what do you think?

@jsparber
Copy link
Contributor Author

jsparber commented Nov 6, 2023

So you're proposing a window icon change. I'm not sure that's allowed... @john-preston what do you think?

No it's not. It just uses the symbolic icon in specific places, defined by the platforms theme. For example currently in GNOME the hicolor icon is used in the top bar but made monochrome, because it Telegram doesn't provide a symbolic variant and it would look bad on the platform.

How it will look like
image

@ilya-fedin
Copy link
Contributor

You say it's not but I see on your screenshot that the window icon is changed

@jsparber
Copy link
Contributor Author

jsparber commented Nov 6, 2023

You say it's not but I see on your screenshot that the window icon is changed

This is not the window icon, it's the icon used in the topbar. I mean it's introducing a symbolic variant not a new icon.

Btw this is how it looks currently:
image

@ilya-fedin
Copy link
Contributor

This is the same thing, it's just you call it an other way

@john-preston
Copy link
Member

I've added our svg icon to the repository:

https://github.com/telegramdesktop/tdesktop/blob/dev/Telegram/Resources/icons/tray_monochrome.svg

Maybe it could be used instead?

@ilya-fedin
Copy link
Contributor

Please rebase rather than merging

GNOME Shell uses the symbolic icon in the topbar and notifications.
@jsparber
Copy link
Contributor Author

jsparber commented Nov 6, 2023

Please rebase rather than merging

Sorry about that. I updated the commit using the icon you added.

@john-preston john-preston merged commit 642b0ed into telegramdesktop:dev Nov 7, 2023
9 checks passed
@ilya-fedin
Copy link
Contributor

@john-preston you maybe (unless you don't want this for official builds) merged this too early as the PR doesn't contain code to install the icon for official build

@jsparber
Copy link
Contributor Author

jsparber commented Nov 7, 2023

@john-preston you maybe (unless you don't want this for official builds) merged this too early as the PR doesn't contain code to install the icon for official build

what do you mean?

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Nov 7, 2023

Official build copies the icons in runtime with the code in specific_linux.cpp. This cmake code is no-op for the official build.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants