D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. How would we add the separator role to color scheme files if separator color was added to upstream Qt? Wouldn't we still need to add a separator role to KColorScheme so that we could map the color in the color scheme file to the equivalent QPalette ColorRole?

D25814: [KColorScheme] Add SeparatorColor

ndavis abandoned this revision. ndavis added a comment. Abandoning this since others would rather have this in QPalette REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc: ahiemstra, broulik, manueljlin, alexde, ngraham,

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. OK, I can agree with contributing upstream. I'll try talking on that bug report. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc: ahiemstra, broulik, manueljlin, alexde, ngraham, davidedmundson,

D25814: [KColorScheme] Add SeparatorColor

davidedmundson added a comment. Relevant link on that last comment: https://bugreports.qt.io/browse/QTBUG-63331 They actively seeked our opinion on colour roles REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc:

D25814: [KColorScheme] Add SeparatorColor

cfeck added a comment. There is QPalette::Button, but I don't see any hover/focus colors in QPalette. If we want more roles, we seriously need to contribute them upstream. Qt 6 is a chance to avoid diverging more. REPOSITORY R265 KConfigWidgets REVISION DETAIL

D25814: [KColorScheme] Add SeparatorColor

ahiemstra added a comment. I actually ran into this with the new Card designs I implemented in Kirigami. Kirigami right now does a lot of `tint(backgroundColor, textColor)` to generate separator colours. However, this leads to potential differences when the color set is View vs. Window or

D25814: [KColorScheme] Add SeparatorColor

ngraham added a comment. I have to agree with @ndavis here. Adding a separator color to the color scheme seems sensible enough to me, for the reasons previously provided. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc:

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#598380 , @cfeck wrote: > > Why don't focus and hover colors belong? > > Because an application cannot know if and how a style does indicate focus or hovering. I don't understand this objection. What

D25814: [KColorScheme] Add SeparatorColor

cfeck added a comment. > Why don't focus and hover colors belong? Because an application cannot know if and how a style does indicate focus or hovering. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc: broulik,

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. I think this is still ultimately the right thing to do. Making the frame/separator color a mix of 2 other colors makes it harder to get just the right color for both the light and dark variations of a theme unless you design the color palettes to solve problems in

D25814: [KColorScheme] Add SeparatorColor

ndavis added a subscriber: broulik. ndavis added a comment. In D25814#591053 , @ngraham wrote: > I wonder if a path forward is to move this option into Breeze itself and keep it out of the color scheme. We would add an option for "dark

D25814: [KColorScheme] Add SeparatorColor

ngraham added a comment. I wonder if a path forward is to move this option into Breeze itself and keep it out of the color scheme. We would add an option for "dark separators when using a dark color scheme" in the super hidden Breeze settings window. REPOSITORY R265 KConfigWidgets

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#574395 , @filipf wrote: > Already asked in VDG, but repeating here: how would we handle dark separators on really dark color schemes? > > This is the most downloaded color scheme and it won't work well with

D25814: [KColorScheme] Add SeparatorColor

filipf added a comment. In D25814#574389 , @ngraham wrote: > In D25814#574380 , @ndavis wrote: > > > FWIW, Adwaita Dark uses dark separators, but GNOME doesn't use nearly as many icons as we do.

D25814: [KColorScheme] Add SeparatorColor

ngraham added a comment. In D25814#574380 , @ndavis wrote: > FWIW, Adwaita Dark uses dark separators, but GNOME doesn't use nearly as many icons as we do. F7806761: Screenshot_20191209_155912.png

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#574356 , @hpereiradacosta wrote: > I meant: do the "light" (with window text color) toolbar icons play well with dark toolbar separators as opposed to current semi-light toolbar separators I assume you

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a comment. > I'm not sure what you mean when you wonder how it would play with monochrome icons. It should have nothing to do with them unless you embed stylesheets in the SVGs to use the separator color in breeze icons. I meant: do the "light" (with window

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#574250 , @hpereiradacosta wrote: > - You should add toolbar separators, tabboxes and group boxes to that. For toolbar separators, I wonder how this would play with monochrome (color-themed) icons. I'm

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a comment. In D25814#574197 , @ndavis wrote: > In D25814#574160 , @davidedmundson wrote: > > > Can we clarify what separators we're referring to. > > > Frames around UI

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a comment. > So the background here is that we've gotten a new VDG designer who produced mockups of Breeze dark with dark separators, and some people absolutely fell in love with them, while other people hated them. We could not achieve consensus on moving to use

D25814: [KColorScheme] Add SeparatorColor

ngraham added a comment. In D25814#574197 , @ndavis wrote: > In D25814#574160 , @davidedmundson wrote: > > > Can we clarify what separators we're referring to. > > > Frames around UI elements

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#574129 , @hpereiradacosta wrote: > Few more comments on this: > > - general: you will never be able to make all the opiniated people happy, and you have to draw a line (otherwise your code will become

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. In D25814#574160 , @davidedmundson wrote: > Can we clarify what separators we're referring to. Frames around UI elements and horizontal/vertical lines that separate UI elements. These are the most common

D25814: [KColorScheme] Add SeparatorColor

davidedmundson added a comment. Can we clarify what separators we're referring to. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc: davidedmundson, filipf, cfeck, hpereiradacosta, kde-frameworks-devel, LeGast00n, GB_2,

D25814: [KColorScheme] Add SeparatorColor

cfeck added a comment. I also agree with the concerns rised by Hugo. An application will use various frame primitives, and the style decides how to draw them. It doesn't belong in the API, though (but neither do Focus and Hover colors). REPOSITORY R265 KConfigWidgets REVISION DETAIL

D25814: [KColorScheme] Add SeparatorColor

dfaure resigned from this revision. dfaure added a comment. This revision now requires review to proceed. Hugo's arguments make sense, removing my approval. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg Cc: filipf, cfeck,

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a comment. Few more comments on this: - general: you will never be able to make all the opiniated people happy, and you have to draw a line (otherwise your code will become bloated, buggy, and unmaintainable) - regarding this specific case: many widget style will

D25814: [KColorScheme] Add SeparatorColor

filipf added a comment. I think this is useful. Every once in a while we get opinionated people who think light separators are awful in the Breeze Dark scheme (I would hate dark ones on the other hand) or even some ricer people who want to turn off separators (which they could do now by

D25814: [KColorScheme] Add SeparatorColor

ndavis added a comment. @hpereiradacosta, Fair points and I'm glad you spoke up. JFYI, I'm in no rush to land this and I will consider reserving this change for KF6 if experienced KDE devs think that is best. In D25814#574105 ,

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a subscriber: cfeck. hpereiradacosta added a comment. Would also be good to have the opinion of @cfeck on this. REPOSITORY R265 KConfigWidgets BRANCH separator-color (branched from master) REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis,

D25814: [KColorScheme] Add SeparatorColor

hpereiradacosta added a comment. Adding new entries to the kcolorscheme should be done with a lot of care, because it could be seen as some sort of API break for existing colorscheme, as soon as you start using this color in the widget style: you would need a fallback implementation, for

D25814: [KColorScheme] Add SeparatorColor

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Code looks OK. No opinion on usefulness though. REPOSITORY R265 KConfigWidgets BRANCH separator-color (branched from master) REVISION DETAIL https://phabricator.kde.org/D25814 To:

D25814: [KColorScheme] Add SeparatorColor

ndavis updated this revision to Diff 71096. ndavis added a comment. - update @since version REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25814?vs=71095=71096 BRANCH separator-color (branched from master) REVISION DETAIL

D25814: [KColorScheme] Add SeparatorColor

ndavis edited the summary of this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25814: [KColorScheme] Add SeparatorColor

ndavis edited the summary of this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25814: [KColorScheme] Add SeparatorColor

ndavis edited the summary of this revision. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25814: [KColorScheme] Add SeparatorColor

ndavis edited the summary of this revision. ndavis added reviewers: Frameworks, VDG, dfaure. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D25814 To: ndavis, #frameworks, #vdg, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25814: [KColorScheme] Add SeparatorColor

ndavis created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ndavis requested review of this revision. REVISION SUMMARY Users sometimes ask for the ability to customize the color of separators. When talking about how the next version of