Allow users to disable bird view
Needs RevisionPublic

Authored by juchatycapelle on Nov 2 2019, 7:06 AM.

Details

Reviewers
ngraham
Group Reviewers
VDG
Gwenview
Summary

The bird's eye view is actually displayed over the content of the image when the image is zoomed in. While this is a useful feature it can in some circumstances be a problem for the user. For example this is annoying when reading comics.

This patch adds an option to the view menu to disable the bird's eye view.

related to feature request BUG 413697

Test Plan

I tested opening the program after enabling or disabling the bird's eye view option. Changes were persistent and the display was updated accordingly.
I successfuly opened multiple images at the same time, the option was applied on each one of them. I was also able to change the option on the fly when displaying one or more images.

Diff Detail

Repository
R260 Gwenview
Lint
Lint Skipped
Unit
Unit Tests Skipped
juchatycapelle created this revision.Nov 2 2019, 7:06 AM
Restricted Application added a project: Gwenview. · View Herald TranscriptNov 2 2019, 7:06 AM
juchatycapelle requested review of this revision.Nov 2 2019, 7:06 AM
ngraham requested changes to this revision.Nov 2 2019, 7:56 PM
ngraham added a reviewer: VDG.
ngraham added a subscriber: ngraham.

Thanks for the patch! The functionality seems to work fine. However it appears to cause a layout repression in the settings view:

That needs to be fixed at the minimum. Also, please refer to this thie feature/setting as "Bird's eye view."

In the future, when submitting patches that change the user interface, please make sure to add a screenshot or video to the Test Plan section, and add VDG as a reviewer.

I'm not necessarily opposed to this, but Gwenview's Image View settings page is already quite cluttered with options. I wonder if there is a better way to toggle this on or off. Perhaps the checkbox could be located in the toolbar below the image view, or in a new overflow menu button we could add on the right side of that toolbar.

Alternatively, if living in the Image View page makes the most sense, we should probably condense the view by replacing some or all of those multi-item radio buttons with comboboxes, which will save a lot of space.

Thoughts, VDG folks?

This revision now requires changes to proceed.Nov 2 2019, 7:56 PM
ndavis added a subscriber: ndavis.Nov 2 2019, 8:36 PM

I'm not necessarily opposed to this, but Gwenview's Image View settings page is already quite cluttered with options. I wonder if there is a better way to toggle this on or off. Perhaps the checkbox could be located in the toolbar below the image view, or in a new overflow menu button we could add on the right side of that toolbar.

Alternatively, if living in the Image View page makes the most sense, we should probably condense the view by replacing some or all of those multi-item radio buttons with comboboxes, which will save a lot of space.

Thoughts, VDG folks?

I assume that this patch is for controlling this floating thumbnail:

Maybe it could be an option in the View menu? This seems like something people may want to toggle on and off with a keyboard shortcut (remembering the state from previous instances) rather than opening the settings dialog to enable/disable.

Maybe it could be an option in the View menu? This seems like something people may want to toggle on and off with a keyboard shortcut (remembering the state from previous instances) rather than opening the settings dialog to enable/disable.

Yes, that's an excellent idea.

Thanks for the patch! The functionality seems to work fine. However it appears to cause a layout repression in the settings view

Oops ! I forgot to increment some row numbers, I checked a little bit quickly.

Maybe it could be an option in the View menu? This seems like something people may want to toggle on and off with a keyboard shortcut (remembering the state from previous instances) rather than opening the settings dialog to enable/disable.

Let's do this.

juchatycapelle edited the summary of this revision. (Show Details)
juchatycapelle edited the test plan for this revision. (Show Details)

I took a little bit longer. I moved the option in the View menu. I still use the configuration to make the changes persistent.
The ViewMainPage class is in charge of handling the signals from the menu. It was necessary to do the initial handling on a top level view so that changes can be handled at any time, lower level views are not guaranted to be loaded.

I used the Qt signal mechanism to forward menu signals to the ViewMainPage class, which forwards them to the DocumentView class, which in turn forwards them to the BirdEyeView class.
When a menu signal is raised the ViewMainPage class first update the configuration then forward the signal.

Since the BirdEyeView objects are only created upon URL opening from ViewMainPage I decided to set the BirdEyeView status by raising a signal to the matching DocumentView after each openUrl call.
I didn't want to use the configuration class in parallel of the singal mechanism because I was affraid of possible race conditions.

The BirdEyeView class now has an "enabled" field which default to true upon creation and is overriden by the signal following.

ngraham requested changes to this revision.Dec 3 2019, 6:56 PM

Thanks, looking better! When you edit app/gwenviewui.rc, you need to bump the version number that's at the top of the file. In addition, I added some inline comments that need to be addressed:

app/viewmainpage.cpp
258 ↗(On Diff #70195)

and here

471 ↗(On Diff #70195)

and here

474 ↗(On Diff #70195)

Add an action verb and use Title Case:: "Show Bird's Eye View"

app/viewmainpage.h
136 ↗(On Diff #70195)

there are extra spaces here

155 ↗(On Diff #70195)

and here

lib/documentview/birdeyeview.cpp
73

and here

96

and here

98

and here

215

Indentation

lib/documentview/documentview.cpp
468 ↗(On Diff #70195)

and here

673 ↗(On Diff #70195)

Move the { to the next line

lib/documentview/documentview.h
153 ↗(On Diff #70195)

and here

191 ↗(On Diff #70195)

and here

lib/gwenviewconfig.kcfg
155

and here

This revision now requires changes to proceed.Dec 3 2019, 6:56 PM