D21027: [Notes] Fix icons being almost invisible with light backgrounds when using dark themes

This revision was automatically updated to reflect the committed changes. Closed by commit R114:84742d8965bd: [Notes] Fix icons being almost invisible with light backgrounds when using dark… (authored by filipf). REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf marked 3 inline comments as done. filipf added a comment. Should work fairly well now with the 2 dependencies. REPOSITORY R114 Plasma Addons BRANCH background-appropriate-icon-colors (branched from master) REVISION DETAIL https://phabricator.kde.org/D21027 To: filipf, #plasma,

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 59286. filipf added a comment. - use readonly property instead of function - make icon sizes scalable REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57785=59286 BRANCH

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf added a comment. @broulik I tried again in master and `icon.width` and `icon.height` aren't working for me with this patch. REPOSITORY R114 Plasma Addons BRANCH background-appropriate-icon-colors (branched from master) REVISION DETAIL https://phabricator.kde.org/D21027 To:

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf added a comment. In D21027#464443 , @broulik wrote: > > I tried to set icon.width and icon.height but it doesn't do anything. > > In desktop style that should work since D20418 Tried it in

D21027: [Notes] Fix icons being almost invisible with light backgrounds

broulik added a comment. > I tried to set icon.width and icon.height but it doesn't do anything. In desktop style that should work since D20418 REPOSITORY R114 Plasma Addons BRANCH background-appropriate-icon-colors (branched from master)

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf added inline comments. INLINE COMMENTS > broulik wrote in main.qml:258 > Not too happy of using desktop buttons here instead of plasma components but > custom color stuff doesn't work in Plasma. > However, given it's a tool button and this solves a genuine usability problem > I think

D21027: [Notes] Fix icons being almost invisible with light backgrounds

broulik added inline comments. INLINE COMMENTS > main.qml:63 > +// an unknown colour over a known colour is a bad move as you end up > with white on yellow. > +function textIconColor() { > +if (plasmoid.configuration.color === "black" || > plasmoid.configuration.color ===

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 57785. filipf added a comment. - add space after comment slashes - add punctuation in the comments for the added function REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57776=57785 BRANCH

D21027: [Notes] Fix icons being almost invisible with light backgrounds

ngraham added inline comments. INLINE COMMENTS > main.qml:60 > > +//define colors used for icons in ToolButtons and for text in TextArea > +//this is deliberately _NOT_ the theme color as we are over a known > bright background Add a space between the slashes and the text. Also since

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 57776. filipf added a comment. remove some hack I was working on for scrollbar color REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57775=57776 BRANCH background-appropriate-icon-colors (branched from

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 57775. filipf added a comment. write a function for text and icon colors so as to not duplicate code everywhere REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57578=57775 BRANCH

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf planned changes to this revision. filipf added a comment. In D21027#461230 , @ngraham wrote: > Gotcha. At least use a function instead of duplicating the same code in each button. Ah yeah, rereading your original comment I now see

D21027: [Notes] Fix icons being almost invisible with light backgrounds

ngraham added a comment. In D21027#461027 , @filipf wrote: > I get what you're saying but that's just how the applet is designed. It's preconfigured SVG backgrounds (you can find them in

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf added a subscriber: broulik. filipf added a comment. I get what you're saying but that's just how the applet is designed. It's preconfigured SVG backgrounds (you can find them in `/usr/share/plasma/desktoptheme/default/widgets/notes.svgz`). I'm not really introducing any hardcoding

D21027: [Notes] Fix icons being almost invisible with light backgrounds

ngraham added a comment. I don't like hardcoding the things we're comparing against. If we ever add more colors besides "black" or "translucent-light", or of the actual color values of those change (unlikely, but possible), all these buttons will need their code to be adjusted and that'll

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 57578. filipf added a comment. move accessibleName back into ToolButton, but still refer to the Tooltip's text REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57577=57578 BRANCH

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf updated this revision to Diff 57577. filipf added a comment. move accessibleName into tooltip and refer to the tooltip's text REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21027?vs=57576=57577 BRANCH background-appropriate-icon-colors

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf added a comment. In D21027#460966 , @davidedmundson wrote: > Plasma has this "ColorScope" thing. Does this work as a solution? Will fix the frame Seems like that could be one of the solutions:

D21027: [Notes] Fix icons being almost invisible with light backgrounds

davidedmundson added a comment. Plasma has this "ColorScope" thing. Does this work as a solution? Will fix the frame REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D21027 To: filipf, #plasma, #vdg, ngraham Cc: davidedmundson, plasma-devel, jraleigh, GB_2,

D21027: [Notes] Fix icons being almost invisible with light backgrounds

filipf created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. filipf requested review of this revision. REVISION SUMMARY Old code had no way to adapt icon colors based on background so this patch uses different components to color the icons black with