Introduce the Header color set
ClosedPublic

Authored by mart on Apr 27 2020, 3:31 PM.

Details

Summary

This introduces two concepts: a new color set for Header (or: titlebar?
open question)
and the possibility for a color set to have a different config group (a
sub group) for different states (there, for inactive)

the Header area should probably also going to slowly relace the WM
section of color themes

This is needed for implement properly T10201, this graphical union
between the titlebar and the toolbar.

There is a problem of NColorSets that has been recently added,
because it makes impossible to hard/impossible to
properly add a set in a 100% abi compatible way, strictly speaking it
should be compatible tough keeping NColorSets as the last value, is a
kindof behavior change, though *should* be acceptable (old users would
get the Header value as NColorSets but everything working never the less)

Test Plan

doing a color set with Header group, the proper values get read

Diff Detail

Repository
R265 KConfigWidgets
Branch
arcpatch-D29232
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27046
Build 27064: arc lint + arc unit
mart created this revision.Apr 27 2020, 3:31 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 27 2020, 3:31 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mart requested review of this revision.Apr 27 2020, 3:31 PM
mart retitled this revision from Introduce the Tools color set to [WIP][RFC]Introduce the Tools color set.Apr 27 2020, 3:32 PM
cblack accepted this revision.Apr 27 2020, 3:35 PM
cblack added a subscriber: cblack.

+1 to the approach here. I assume this would probably replace the WM set in kdeglobals for all practical usage?

This revision is now accepted and ready to land.Apr 27 2020, 3:35 PM
ndavis added a subscriber: ndavis.Apr 27 2020, 4:15 PM

I personally think it should be Header since Tools sound ambiguous and oddly limited to anyone unfamiliar with the context behind this patch

mart updated this revision to Diff 81363.Apr 27 2020, 4:16 PM

call it Header

ndavis added inline comments.Apr 27 2020, 4:19 PM
src/kcolorscheme.h
127
  • thould
  • titlebar titlebar
ndavis added inline comments.Apr 27 2020, 4:21 PM
src/kcolorscheme.h
133

I think it would be good to explain the issue about enums being converted into plain integers here so that people know why they shouldn't depend on it. Also, is there a way to hide this from the api.kde.org page for kcolorscheme?

mart retitled this revision from [WIP][RFC]Introduce the Tools color set to [WIP][RFC]Introduce the Header color set.Apr 27 2020, 4:23 PM
mart edited the summary of this revision. (Show Details)
mart edited the test plan for this revision. (Show Details)
mart requested review of this revision.Apr 27 2020, 4:25 PM
cblack added inline comments.Apr 27 2020, 6:22 PM
src/kcolorscheme.h
133

#ifndef K_DOXYGEN

davidre added inline comments.
src/kcolorscheme.cpp
271

I would have expected at least of one the new colors to have the same value as this one.

ndavis added inline comments.Apr 27 2020, 9:12 PM
src/kcolorscheme.cpp
271

Why is that? The only one that might is Active, but that's not really used much right now.

davidre added inline comments.Apr 28 2020, 6:58 AM
src/kcolorscheme.cpp
271

Because the new colors are the replacement for theses colors. I thought one of those might map to this

mart added inline comments.Apr 28 2020, 9:09 AM
src/kcolorscheme.cpp
271

i also added a question on the task, whether final colors are decided for it, they should go up here
https://phabricator.kde.org/T10201

src/kcolorscheme.cpp
258

227, 229, 231

259

239, 240, 241

260

35, 38, 41

261

112, 125, 138

262

61, 174, 233

263

41, 128, 185

264

155, 89, 182

265

218, 68, 83

266

246, 116, 0

267

39, 174, 96

mart updated this revision to Diff 83090.Wed, May 20, 2:18 PM
  • update to new colors
mart retitled this revision from [WIP][RFC]Introduce the Header color set to Introduce the Header color set.Wed, May 20, 2:19 PM
mart marked 9 inline comments as done.
ndavis accepted this revision.Wed, May 20, 2:20 PM
This revision is now accepted and ready to land.Wed, May 20, 2:20 PM
mart closed this revision.Wed, May 20, 2:24 PM
meven added a subscriber: meven.Wed, May 20, 2:58 PM

@mart you have a couple of typos left and recommendation to add some doc.