Review Request 110505: set KItemListView m_styleOption.palette from scenes first view

[Thomas Lübking]

> On May 23, 2013, 11:10 a.m., Frank Reininghaus wrote:
> > Thanks for the feedback, but I must admit that I still do not understand at all what the actual problem is.
> > 
> > My comments here might be really stupid - sorry if that is the case, but I have neither written nor ever worked with the code that you are proposing to change, and as I said, I have basically no clue about anything that is related to colors. Therefore, I prefer to understand what problem any proposed patch is trying to solve.
> > 
> > I had assumed that you found a problem with the current code that can somehow be triggered by changing the colors in the System Settings or by interacting with Dolphin or some other part of the desktop in some way. Is that correct? If yes, please describe what this problem is. I have tried to change the colors in the settings, but I was unable to see any difference with and without your patch (I can reproduce the problem that color changes at runtime are not handled well, but this is not the point of your patch).
> > 
> > If my assumption is wrong and you are just trying to make things more consistent for the case that a scene has a custom palette (and that case does currently not happen in Dolphin), please say so. I'm fine with such a change as well, I just want to understand what you are trying to achieve with the patch.
> > 
> > Thanks and sorry again if my comments are stupid.

As long as the class is used in dolphin exclusively, the only "outside" trigger for the particular error are GUI styles which manipulate the palette during (parenting) widget polishment, eg. to invert docks.

The original issue here is that the design completely unexports color management and the style has no chance to consider it in its actions.

Should the class be moved into libs to be used in other places like filedialogs (and this probably already affects the kpart), not even the direct client code (the application) has a chance to manipulate colors, since -like with "naive" QSS usage- always the Application palette is used.

The more global error of "the palette is assigned once from the application palette and never touched again" (caused by eg. inverting the colorscheme and signalling that down via IPC from "kcmshell colors") is not addressed by this particular patch.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110505/#review33028
-----------------------------------------------------------


On May 22, 2013, 9:46 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110505/
> -----------------------------------------------------------
> 
> (Updated May 22, 2013, 9:46 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The palette is currently always constructed empty, thus defaults to the application general palette what fails if the view gets a different palette.
> This is not "perfect" as it will not catch palette updates at runtime, but "works for me" (inverting docks at style polishment)
> 
> If QGraphicsWidget::palette() was used it would probably be possible to update the palette (via QGraphicsWidget::scene())
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.h 6d609a9 
>   dolphin/src/kitemviews/kitemlistview.cpp d009655 
> 
> Diff: http://git.reviewboard.kde.org/r/110505/diff/
> 
> 
> Testing
> -------
> 
> yes, getting readable text instead black-on-black
> 
> 
> File Attachments
> ----------------
> 
> PoC
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/05/22/__proof.diff
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130523/bc92d6fc/attachment.htm>