D18940: Add battery icons

This revision was automatically updated to reflect the committed changes. Closed by commit R266:b04959059668: Add battery icons (authored by ndavis). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18940?vs=51712&id=51730 REVISION DETAIL https://phabrica

D18940: Add battery icons

ndavis edited the summary of this revision. REPOSITORY R266 Breeze Icons BRANCH battery-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D18940 To: ndavis, #vdg, hein, ngraham Cc: broulik, ngraham, filipf, trickyricky26, kde-frameworks-devel, hein, michaelh, bruns

D18940: Add battery icons

ndavis updated this revision to Diff 51712. ndavis added a comment. Change `status/22` and `status/22@2x` from Scalable to Fixed REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18940?vs=51462&id=51712 BRANCH battery-icons (branched from master) REVI

D18940: Add battery icons

ndavis added a comment. In D18940#412125 , @ngraham wrote: > In D18940#411818 , @ndavis wrote: > > > Would changing `status/22` to Fixed cause a problem? It doesn't appear to cause any real issue whe

D18940: Add battery icons

ngraham added a comment. In D18940#411818 , @ndavis wrote: > Would changing `status/22` to Fixed cause a problem? It doesn't appear to cause any real issue when I look at the change through Cuttlefish. Run dolphin with `QT_SCALE_FACTOR=2`

D18940: Add battery icons

ndavis added a comment. This is the configuration for `status/32`: [status/32] Size=32 Context=Status Type=Fixed This currently prevents the 32px icons in this patch from being used: [status/22] Size=22 Context=Status Type=Scalable MinSize=22 M

D18940: Add battery icons

ngraham added a comment. Not trying to slip anything in (this patch doesn't and can't touch plasma-framework, after all), just mentioning it. :) The proposal is in T10046: Improve the colors, color consistency and colorscheme compatibility of Breeze .

D18940: Add battery icons

broulik added a comment. The do a proper proposal (I bet you already have a task about that) instead of just slipping things in one by one in unrelated reviews REPOSITORY R266 Breeze Icons BRANCH battery-icons (branched from master) REVISION DETAIL https://phabricator.kde.org/D18940

D18940: Add battery icons

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Ideally I would like to move all icons out of Plasma themes, because these themes are not 100% self-contained and often use icons from the icon theme anyway, and users get confused as to

D18940: Add battery icons

ndavis added a comment. In D18940#411319 , @broulik wrote: > The battery icon is composited of multiple SVG items layered ontop of each other, it doesn't just load a single icon. > > I'm in favor of adding proper battery icons to the Breeze i

D18940: Add battery icons

broulik added a comment. The battery icon is composited of multiple SVG items layered ontop of each other, it doesn't just load a single icon. I'm in favor of adding proper battery icons to the Breeze iconset, definitely need "battery", "battery-caution", "battery-low" icons (from the fr

D18940: Add battery icons

ndavis added a comment. In D18940#411317 , @broulik wrote: > In D18940#411316 , @ndavis wrote: > > > Won't 3rd party themes contain their own icons? > > > Sure but in the style of a Plasma batte

D18940: Add battery icons

broulik added a comment. In D18940#411316 , @ndavis wrote: > Won't 3rd party themes contain their own icons? Sure but in the style of a Plasma battery.svgz, so if we change the `BatteryIcon` to use icon theme instead of Plasma theme icons

D18940: Add battery icons

ndavis added a comment. In D18940#411303 , @broulik wrote: > > maybe we can port the code to use these as icons from the icon theme rather than SVGs from the Plasma theme and then delete them from the Plasma theme > > And in the process brea

D18940: Add battery icons

broulik added a comment. > maybe we can port the code to use these as icons from the icon theme rather than SVGs from the Plasma theme and then delete them from the Plasma theme And in the process break all third party Plasma themes? REPOSITORY R266 Breeze Icons REVISION DETAIL http

D18940: Add battery icons

ndavis added a comment. In D18940#411154 , @ngraham wrote: > So I guess this means some duplication between the Breeze icon theme and the Breeze Plasma theme for a while, right? Not 100%. I need to make a device icon for an uninterruptibl

D18940: Add battery icons

ngraham added a comment. So I guess this means some duplication between the Breeze icon theme and the Breeze Plasma theme for a while, right? If we get this in before Frameworks 5.58, then Plasma 5.16 can depend on having them in the Breeze icon theme and maybe we can port the code to us

D18940: Add battery icons

trickyricky26 added a comment. In D18940#410980 , @ndavis wrote: > I think having a different location based on battery level would get annoying You're probably right. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator

D18940: Add battery icons

ndavis added a comment. In D18940#410649 , @trickyricky26 wrote: > I like it! > Would you consider moving the green charging to either the filled-in or the white part where the symbol is currently above the line where the filled-in part ends

D18940: Add battery icons

ndavis added a comment. In D18940#410688 , @filipf wrote: > Nice. Shouldn't the plug here be green, same as everywhere else? > > F6611280: image.png This was fixed in the last change REPOSITORY

D18940: Add battery icons

filipf added a comment. Nice. Shouldn't the plug here be green, same as everywhere else? F6611280: image.png REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D18940 To: ndavis, #vdg, hein Cc: filipf, trickyricky26, kde-f

D18940: Add battery icons

trickyricky26 added a comment. I like it! Would you consider moving the green charging to either the filled-in or the white part where the symbol is currently above the line where the filled-in part ends? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D18940

D18940: Add battery icons

ndavis edited the summary of this revision. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D18940 To: ndavis, #vdg, hein Cc: kde-frameworks-devel, hein, michaelh, ngraham, bruns

D18940: Add battery icons

ndavis updated this revision to Diff 51462. ndavis added a comment. Fix missing PositiveText stylesheet for battery-020-charging REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18940?vs=51461&id=51462 BRANCH battery-icons (branched from master) REVI

D18940: Add battery icons

ndavis added a comment. Not certain if I've got the `index.theme`s right REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D18940 To: ndavis, #vdg, hein Cc: kde-frameworks-devel, hein, michaelh, ngraham, bruns

D18940: Add battery icons

ndavis updated this revision to Diff 51461. ndavis added a comment. Add `status/32` to `index.theme` REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18940?vs=51460&id=51461 BRANCH battery-icons (branched from master) REVISION DETAIL https://phabri

D18940: Add battery icons

ndavis edited the test plan for this revision. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D18940 To: ndavis, #vdg, hein Cc: kde-frameworks-devel, hein, michaelh, ngraham, bruns

D18940: Add battery icons

ndavis created this revision. ndavis added reviewers: VDG, hein. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ndavis requested review of this revision. REVISION SUMMARY Adds battery icons at 16, 22 and 32 px. Adds `status/32` to `index.theme`. Includes sy