Navigation context uses theme color.
Needs ReviewPublic

Authored by tristanp on Jun 29 2019, 8:46 PM.


Group Reviewers

Previously the navigation context (tool displaying information about
code) was using fix white color theme. This was nice for people using
light themes but for dark themes it looked like a lighhouse in the night.

The solution proposed is to use colors from the common theme for
background but also for text and code displayed in the tool panel.
These colors are supposed to be contrasted enought and readable.

Modifications are limited to AbstractNavigationContext and sub classes.
AbstractNavigationContext hold a ColorizerTheme containing colors
named as before (m_typeHighlight, m_positionHighlight, ...), this theme
is retrieved from AbstractNavigationContext::theme() and every usage
of xxxHiglight(string) is replaced by theme().m_xxxHiglight(string).

Theme initialisation is proceeded into AbstractNavigationContext constructor
with a KColorScheme.

Note: This is my second patch in this projet, please let me know if I miss anything.

Diff Detail

R32 KDevelop
No Linters Available
No Unit Test Coverage
Build Status
Buildable 13454
Build 13472: arc lint + arc unit
tristanp created this revision.Jun 29 2019, 8:46 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJun 29 2019, 8:46 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
tristanp requested review of this revision.Jun 29 2019, 8:46 PM
tristanp edited the summary of this revision. (Show Details)Jun 29 2019, 8:47 PM
tristanp added a reviewer: KDevelop.
apol added a subscriber: apol.Jun 30 2019, 8:26 AM

Patch looks good to me. Would you be able to offer screenshots using Breeze?


Given they're public API, I would remove the m_ prefix.

tristanp updated this revision to Diff 60904.Jul 1 2019, 8:36 AM
  • Rename m_xxxHighlight to xxxHiglight as they are public.

Interesting approach, might work out, thanks for doing this :)

Not sure yet about all color mappings, that might need some more thinking, at least on a quick test across themes that resulted in quite some unreadable variants. Will play a bit during the upcoming WE.

One thing which might be nice to have added (later) is an automatic update on color scheme changes, which would be at least useful for the Code Browser tool view to not look out of place on a change (especially important on testing color schemes ;) ).


m_typeHighlight -> typeHighlight

Not sure yet about all color mappings, that might need some more thinking, at least on a quick test across themes that resulted in quite some unreadable variants. Will play a bit during the upcoming WE.

Ah, no, I confused myself with the used KColorScheme object of the running application not updating to colorscheme/palette changes of the UI. With a WIP prototype addition to update on UI colorscheme changes at runtime things look better or rather fine.

Will play some more and then propose the update mechanism as addition. Might be an idea to accept this patch already as is, for people not changing colorscheme at runtime ;)

kossebau added inline comments.Jul 8 2019, 1:13 PM

This comment though still applies, as this is about HTML injected from the documentation, e.g. about C++ classes.
Though admittedly this is currently partially broken at least for Qt documentation for me, will need testing for other documentation first.

tristanp added inline comments.Aug 2 2019, 6:57 PM

What palette is used with by default ? Does it follow the KColorScheme ?

Would it be possible and interesting to reuse code highlighting for the code quote ?

mwolff added a subscriber: mwolff.Oct 7 2019, 8:14 PM

hey! can you also show a screenshot of PHP or Qt documentation showing? these can contain arbitrary HTML and sometimes even contain colored text that expects to be shown on bright backgrounds which would break with this patch. At least that was the case years ago when I worked on this the last time.

due to that, we decided to not use a dark color palette, as it could potentially lead to unreadable text in the documentation popup

if that has changed, then I'm all for it!