Bug 407217

Summary: Toggle Change Colors button does not toggle
Product: [Applications] okular Reporter: Squeaky Pancakes <squeakypancakes>
Component: generalAssignee: Okular developers <okular-devel>
Status: RESOLVED FIXED    
Severity: minor CC: aacid, nate
Priority: NOR    
Version: 1.7.0   
Target Milestone: ---   
Platform: Other   
OS: Linux   
See Also: https://bugs.kde.org/show_bug.cgi?id=173264
Latest Commit: Version Fixed In: 21.08
Sentry Crash Report:
Attachments: Toggle Change Colors not pressed
Toggle Change Colors button pressed
Mockup of newly added Color Mode menu accessed from main toolbar
Screenshot of new Change Colors menu

Description Squeaky Pancakes 2019-05-04 15:35:47 UTC
Created attachment 119840 [details]
Toggle Change Colors not pressed

SUMMARY
When pressing the Toggle Change Colors button it doesn't toggle on in the toolbar.

STEPS TO REPRODUCE
1. Open a document
2. Add Toggle Change Colors button to toolbar
3. Click Toggle Change Colors button

OBSERVED RESULT
Button looks untoggled

EXPECTED RESULT
Button should look like it is toggled

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 5.15.4
KDE Frameworks Version: 5.57.0
Qt Version: 5.12.2

ADDITIONAL INFORMATION
Comment 1 Squeaky Pancakes 2019-05-04 15:36:15 UTC
Created attachment 119841 [details]
Toggle Change Colors button pressed
Comment 2 Laura David Hurka 2019-05-05 13:46:50 UTC
Confirmed, this button’s code was not modified since it was added in commit afc74e76ef. See Bug 173264.

Today, it’s in ui/pageview.cpp:612.

The QAction is not set checkable, which would make sense. In the initialisation of the page view (?), the current state must be read from the config, to initialize the QAction.

Junior Job?
Comment 3 Laura David Hurka 2019-05-08 09:59:11 UTC
Created attachment 119911 [details]
Mockup of newly added Color Mode menu accessed from main toolbar

How about converting “Toggle Change Colors” to a menu, like “View Mode” is? This way, the color mode could be specified from the toolbar. But then, it still wouldn’t appear pressed.

For mockup, see attachment. I also added “Continuous” to the View Mode menu, so View Mode is a completely usable menu for the toolbar. See Bug 407326.
Comment 4 Albert Astals Cid 2019-05-10 21:37:58 UTC
this was added just to have a shorcut if people wanted, wasn't thought that people add it to the toobar, but i guess it would be made to work it for that too
Comment 5 Laura David Hurka 2019-05-11 13:47:19 UTC
Created attachment 119987 [details]
Screenshot of new Change Colors menu

I have secretly created an interesting patch which introduces a new Change Colors menu.

When the Change Colors action is plugged in the toolbar, it creates a checkable QToolButton. I used a modified KActionMenu for this, so it is painted “toggled” when Change Colors is checked.

It is interesting, because it shouldn’t work as far as I understand it yet.
Comment 6 Laura David Hurka 2019-05-11 13:49:00 UTC
(Maybe it’s better to just dump everything besides the left Change Colors button.)
Comment 7 Laura David Hurka 2021-05-29 23:09:01 UTC
Git commit 195bbe3636155613555c27f4e67d21aa1461a5b6 by David Hurka.
Committed on 29/05/2021 at 23:08.
Pushed by davidhurka into branch 'master'.

Create color mode menu.

Implemented using a ColorModeMenu class,
derived from ToggleActionMenu (derived from KActionMenu),
as a child object of PageView.

* KToggleAction for every color mode, allows to set shortcuts for every mode.
  Color mode actions have icons.
* KToggleAction for normal colors mode.
* ToggleActionMenu containing all color mode actions.
  If triggered, toggles color mode between normal colors and last change colors mode.

"Toggle Change Colors" is replaced by "Change Colors", which is actually a toggle action.
Related: bug 437755

M  +2    -1    CMakeLists.txt
M  +7    -0    conf/okular.kcfg
M  +4    -0    conf/okular_core.kcfg
A  +145  -0    part/colormodemenu.cpp     [License: GPL (v2+)]
A  +74   -0    part/colormodemenu.h     [License: GPL (v2+)]
M  +7    -15   part/pageview.cpp
M  +0    -3    part/pageview.h
M  +19   -2    part/part.cpp
M  +1    -0    part/part.h
M  +8    -1    part/preferencesdialog.cpp
M  +2    -0    part/preferencesdialog.h

https://invent.kde.org/graphics/okular/commit/195bbe3636155613555c27f4e67d21aa1461a5b6