Add new activities and virtual desktops icons
ClosedPublic

Authored by GB_2 on Jun 29 2019, 1:59 PM.

Details

Summary

This adds new monochrome icons corresponding to the colorful ones. Also deletes the old monochrome preferences-activies icon and updates the symlinks that pointed to it.


Test Plan

Search for activities and virtual-desktops in Cuttlefish.

Diff Detail

Repository
R266 Breeze Icons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
GB_2 created this revision.Jun 29 2019, 1:59 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 29 2019, 1:59 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
GB_2 requested review of this revision.Jun 29 2019, 1:59 PM
ndavis added a subscriber: ndavis.Jun 29 2019, 4:28 PM

Nice job!

Are there other patches coming soon to fix the code where preferences-activities was used?

GB_2 added a comment.Jun 29 2019, 8:59 PM

Nice job!

Are there other patches coming soon to fix the code where preferences-activities was used?

Yeah, I'll make them soon.

ndavis accepted this revision.Jun 29 2019, 11:57 PM

I'm accepting this, but make sure you don't land this until you've submitted the other patches.

This revision is now accepted and ready to land.Jun 29 2019, 11:57 PM

+1, please mark this patch as a dependency for the other patches and then let's land 'em all together once everything looks good.

Renaming the icon is not quite good, if someone use newer framework and old applications / Plasma will not see an icon. That's pretty happen since framework incorporate new features + bugs fix and Plasma offers LTS.

GB_2 added a comment.Jun 30 2019, 3:57 PM

Renaming the icon is not quite good, if someone use newer framework and old applications / Plasma will not see an icon. That's pretty happen since framework incorporate new features + bugs fix and Plasma offers LTS.

There is no other way, we need to solve T10165. We can backport the changes in Plasma to the LTS branch though.

+1, please mark this patch as a dependency for the other patches and then let's land 'em all together once everything looks good.

Phew, done!

anthonyfieroni added a comment.EditedJun 30 2019, 4:42 PM
In D22155#488497, @GB_2 wrote:

We can backport the changes in Plasma to the LTS branch though.

We can backport but you cannot increase depends on 5.60 to the LTS branch. So it should have a fallback code, if new icon isn't found to be used old one.

https://doc.qt.io/qt-5/qicon.html#fromTheme-1

Renaming the icon is not quite good, if someone use newer framework and old applications / Plasma will not see an icon. That's pretty happen since framework incorporate new features + bugs fix and Plasma offers LTS.

Do you have any examples of distros that do this? To my knowledge, all "LTS" style distros freeze all versions of KDE software, not just Plasma and apps. Though I guess it would be a problem for rolling release distros that will offer users Frameworks 5.60 months before Plasma 5.17.

If it's a problem, we could wait a month and commit the Frameworks change for Frameworks 5.62, which is the release aligned with Plasma 5.17.

GB_2 added a comment.Jun 30 2019, 5:46 PM

Renaming the icon is not quite good, if someone use newer framework and old applications / Plasma will not see an icon. That's pretty happen since framework incorporate new features + bugs fix and Plasma offers LTS.

Do you have any examples of distros that do this? To my knowledge, all "LTS" style distros freeze all versions of KDE software, not just Plasma and apps. Though I guess it would be a problem for rolling release distros that will offer users Frameworks 5.60 months before Plasma 5.17.

If it's a problem, we could wait a month and commit the Frameworks change for Frameworks 5.62, which is the release aligned with Plasma 5.17.

I'd say we land this now for Frameworks 5.60 which releases July 6th and then backport the one small plasma-desktop patch to the stable branch, so it gets in 5.16.3, which releases July 9th.

Renaming the icon is not quite good, if someone use newer framework and old applications / Plasma will not see an icon. That's pretty happen since framework incorporate new features + bugs fix and Plasma offers LTS.

Do you have any examples of distros that do this? To my knowledge, all "LTS" style distros freeze all versions of KDE software, not just Plasma and apps.

IMHO, that should be the *right* scenario, since we don't backport bugfixes in framework, distros that provide LTS should stays on Plasma LTS, applications numbering through release date e.g. 18.04.3 and top of the framework.
+1 to be part of Plasma 5.17

GB_2 added a comment.Jul 3 2019, 8:41 AM

+1 to be part of Plasma 5.17

Wouldn't my proposed approach be better? Also, only one patch out of many actually affects Plasma.

In D22155#488595, @GB_2 wrote:

I'd say we land this now for Frameworks 5.60 which releases July 6th and then backport the one small plasma-desktop patch to the stable branch, so it gets in 5.16.3, which releases July 9th.

GB_2 added a comment.Jul 5 2019, 9:43 AM
This comment was removed by GB_2.

Frameworks 5.60 has been tagged, so this would end up in 5.61 if it lands now. I kind of think we should wait one more month and land it for 5.62 so it's perfectly aligned with Plasma 5.17. Are you okay with that?

GB_2 added a comment.Jul 8 2019, 6:10 AM

Frameworks 5.60 has been tagged, so this would end up in 5.61 if it lands now. I kind of think we should wait one more month and land it for 5.62 so it's perfectly aligned with Plasma 5.17. Are you okay with that?

Yeah, that's fine.

Cool! Consider this a pre-approval to land this at the appropriate time, then.

GB_2 added a comment.Aug 3 2019, 6:18 AM

Actually, it is Frameworks 5.63 that releases on October 12, in time for 5.17 which releases on October 15. I guess I'll need to wait another month.

Guh, I guess the schedule changed. I could have sworn that 5.52 was the original frameworks dependency target.

And in fact it was. :( Looks like that was a typo on the wiki, and 5.62 is the correct one. Feel free to land this right now.

dfaure added a subscriber: dfaure.Sep 7 2019, 9:35 AM

This commit introduced dangling symlinks, detected by make and the CI.

warning: failed to load external entity "./icons-dark/status/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons-dark/status/symbolic/image-loading-symbolic.svg"
warning: failed to load external entity "./icons-dark/actions/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons/status/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons/status/symbolic/image-loading-symbolic.svg"
warning: failed to load external entity "./icons/actions/symbolic/content-loading-symbolic.svg"
gmake[2]: *** [CMakeFiles/breeze-validate-svg.dir/build.make:57: CMakeFiles/breeze-validate-svg] Error 1

icons-dark/status/symbolic/image-loading-symbolic.svg -> ../../actions/16/preferences-activities.svg

Please fix.

GB_2 added a comment.Sep 7 2019, 9:51 AM

This commit introduced dangling symlinks, detected by make and the CI.

warning: failed to load external entity "./icons-dark/status/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons-dark/status/symbolic/image-loading-symbolic.svg"
warning: failed to load external entity "./icons-dark/actions/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons/status/symbolic/content-loading-symbolic.svg"
warning: failed to load external entity "./icons/status/symbolic/image-loading-symbolic.svg"
warning: failed to load external entity "./icons/actions/symbolic/content-loading-symbolic.svg"
gmake[2]: *** [CMakeFiles/breeze-validate-svg.dir/build.make:57: CMakeFiles/breeze-validate-svg] Error 1

icons-dark/status/symbolic/image-loading-symbolic.svg -> ../../actions/16/preferences-activities.svg

Please fix.

Fixed: R266:3935f46b1d5a