D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Henrik Fehlauer]

rkflx accepted this revision.
rkflx added a comment.


  Thanks. Title is still unchanged, but this was more of a hint for future 
contributions, anyway.
  
  Given you are now listed as "KDE developer", I assume you can commit by 
yourself? I guess this should go into the bugfix branch (Applications/17.08), 
then merge to master.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander, rkflx
Cc: rkflx, aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham updated this revision to Diff 19721.
ngraham added a comment.


  Missed one more tiny formatting issue

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7793?vs=19720=19721

REVISION DETAIL
  https://phabricator.kde.org/D7793

AFFECTED FILES
  ui/sidebar.cpp

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: rkflx, aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham updated this revision to Diff 19720.
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.
ngraham added a comment.


  Removed unnecessary parentheses and cleaned up the summary/commit message

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7793?vs=19470=19720

REVISION DETAIL
  https://phabricator.kde.org/D7793

AFFECTED FILES
  ui/sidebar.cpp

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: rkflx, aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Henrik Fehlauer]

rkflx added a comment.


  Great, one item less on my list of things to fix in Okular :)
  
  @ngraham Just a suggestion for future Diffs: Have a look at git's 50/72 rule 
(…of thumb) and the reasoning behind it, as the title of your commit message is 
quite long, really ;-)
  
  Also small coding style nit: superfluous parens could be removed, but not 
really important

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: rkflx, aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Albert Astals Cid]

aacid accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham added a comment.


  Oh I see. Here's how it looks with the Oxygen style:
  
  F3917149: Screenshot_20170918_220609.png 


REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Albert Astals Cid]

aacid added a comment.


  In https://phabricator.kde.org/D7793#145969, @ngraham wrote:
  
  > I did test with Oxygen, as that's a built-in theme. It looks fine:
  >
  > F3911431: Change with Oxygen.png 
  
  
  I don't think that's the oxygen style, that may be the oxygen color theme.
  
  okular -style oxygen
  
  is what i mean

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham added a comment.


  I did test with Oxygen, as that's a built-in theme. It looks fine:
  
  F3911431: Change with Oxygen.png 

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Albert Astals Cid]

aacid added a comment.


  Could you give it a try with the oxygen style?

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid

D7793: Improve text visibility for Okular sidebar tabs that are hovered-over or selected-but-out-of-focus when using Breeze color scheme

[Nathaniel Graham]

ngraham created this revision.
ngraham added a project: Okular.

REVISION SUMMARY
  Changed the colors used from the active theme so that there is never a 
white-on-light-blue situation that makes the text unreadable.
  
  Here are the changes when using the Breeze color scheme:
  
  - When an inactive tab is hovered over, the text is now dark (was light 
before)
  - When a selected tab loses focus, the text is now dark (was light before)
  
  Tested to make sure dark themes do not experience a regression.
  
  BUG: 382139

TEST PLAN
  Tested on KDE Neon (dev unstable). Verified that with the default Breeze 
color scheme, tab text is more readable when hovered over and then active but 
not in focus. See before and after:
  
  F3909143: After.png 
  
  F3909142: Before.png 
  
  Also tested these changes with all standard color schemes shipped with Neon 
to make sure that there were no visual regressions.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7793

AFFECTED FILES
  ui/sidebar.cpp

To: ngraham, aacid, #okular, #kde_applications, broulik, sander
Cc: aacid