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

Rename battery icons, remove legacy battery icons #276

Merged
merged 1 commit into from
May 16, 2022

Conversation

newhoa
Copy link
Contributor

@newhoa newhoa commented Apr 20, 2022

Renames the battery icons to use Upower icon names.
Xfpm has used these since 2015 so I think anything
else can be considered legacy and safe to remove.

Removes legacy xfpm and gpm battery icons and symlinks.

Fixes issue where Power Manager -> Line Power didn't use
the right icon.

@ochosi
Copy link
Member

ochosi commented Apr 22, 2022

Same as with the other big PR - do you know what Mate uses? I don't wanna completely break the theme for users that switch between DEs (even your cleanups are really appreciated!)

@newhoa
Copy link
Contributor Author

newhoa commented Apr 22, 2022

After checking the other DEs on this one, this PR fixes issues in Xfce, Budgie, and Cinnamon. So improvements there.

MATE uses the Upower names for their panel (they use symbolic icons) so it's an improvement there too. But when you go to Panel > Power Indicator > Click > Battery it provides a window with power/battery info. Here they still use the gpm- prefixed icons.

I'm going to open an Issue on mate-power-manager and bring this up. I'd consider it an issue on their part since they've switched some icons to the standard naming but not all. And just looking briefly at the code it looks like they may have intended to use them in all places but aren't.

Let me know what you'd like me to do with this one. They do provide fallback icons with the app so it doesn't appear really broken or without icons or wrong sizes or anything. And it doesn't seem like a thing people would see often (most probably would never). Not sure if it's worth adding 100s of symlinks back in but I'm not sure.

@newhoa
Copy link
Contributor Author

newhoa commented Apr 23, 2022

Just a followup. Looks like the gpm- ones are only used at 48px. So maybe I can add those back in and leave out the rest?

@newhoa newhoa force-pushed the status-battery-cleanup branch from e5da6e7 to 9e0c894 Compare April 24, 2022 13:43
@newhoa newhoa marked this pull request as draft April 25, 2022 04:56
@newhoa
Copy link
Contributor Author

newhoa commented Apr 25, 2022

Switched this to draft till I figure out the MATE thing. It's not as simple as my last reply. Their icon preference/fallback for this is very confusing.

@ochosi ochosi changed the title Rename battery icons, remove legacy battery icons Draft: Rename battery icons, remove legacy battery icons Apr 28, 2022
Renames the battery icons to use Upower icon names.
Xfpm has used these since 2015 (Gnome and Cinnamon
also use these) so I think anything else can be
considered legacy and safe to remove.

Removes legacy xfpm and gpm battery icons and symlinks.

Fixes issue where Power Manager -> Line Power didn't use
the right icon.
@newhoa newhoa force-pushed the status-battery-cleanup branch from 9e0c894 to 20c54ee Compare May 9, 2022 21:45
@newhoa
Copy link
Contributor Author

newhoa commented May 9, 2022

@ochosi Can I get your input on this?

In its current state, this PR is good for Xfce, Gnome, Cinnamon as they all use the same names. It is also an improvement for the MATE status indicator as it will now use a symbolic icon which is easier to see and preferable to the "fake" symbolic icons that the gpm- ones were.

If I add the gpm- icons back in for the mate-power-statistics app, it will force the MATE status indicator/tray icon to go back to a non-symbolic icon.

Unfortunately the mate-power-manager the tray status/indicator and the power statistics window use and prefer different icon names (details here).

So the choice is

  1. Keep this PR as it currently is and have MATE have a nicer more legible tray/status indicator (and let mate-power-statistics use its own provided icons).

  2. Add the gpm- icons back in and have icons for mate-power-statistics but have a less legible tray icon.

Personally I think Power Statistics won't be used often or by many and it would be better to just use the Upower names and have symbolic icons for their tray which will be more useful and seen all the time. It's also good for all the other DEs and makes elementary-xfce slimmer and more manageable.

Let me know what you think. Thanks.

@ochosi
Copy link
Member

ochosi commented May 10, 2022

Thanks for the thorough analysis! I read through the related Mate issues you submitted and believe it should be fixed in their components, not worked around by icon themes.
Also, as you say, the tray/statusnotifier is visible at all times whereas the stats are only visible sometimes. For that the fallback icons are probably good enough.

So I would suggest we merge this as is!

@newhoa newhoa marked this pull request as ready for review May 11, 2022 06:04
@newhoa newhoa changed the title Draft: Rename battery icons, remove legacy battery icons Rename battery icons, remove legacy battery icons May 11, 2022
@ochosi ochosi merged commit f2d06bd into shimmerproject:master May 16, 2022
@newhoa newhoa deleted the status-battery-cleanup branch January 15, 2023 07:39
newhoa added a commit to newhoa/elementary-xfce that referenced this pull request Feb 1, 2025
Some icons in -dark were pointing to icons that are no longer
in the main theme. These are no longer needed.

See shimmerproject#276 (f2d06bd) for more details.
newhoa added a commit that referenced this pull request Feb 1, 2025
Some icons in -dark were pointing to icons that are no longer
in the main theme. These are no longer needed.

See #276 (f2d06bd) for more details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants