Fix issues regarding available operations being out of sync with selected image

Authored by huoni on Mar 4 2018, 2:00 AM.


Group Reviewers

This patch fixes three issues all related to the context manager and item selection model:

Problem 1

Bug: Right clicking an image when no image was previously selected resulted in a mostly unpopulated context menu
Cause: The selectionChanged signal which triggers available image operations to be updated) was queued, and
therefore not emitted before the context menu was shown.
Fix: Do not queue this signal, instead emit it immediately.

Problem 2

Bug: Sometimes the image Operations in the sidebar and toolbar wouldn't activate when selecting an image. This
was always the case when seleting an image using the hover select button when no image was previously selected.
Cause: The URL was only updated when the current index changed. However the currentChanged signal was not
emitted when deselecting all images, or selecting the same image as was previously selected. This resulted in a
borken state where we had an image selected but the current URL was blank.
Fix: Ignore the currentChanged signal on the selection model, and instead rely solely on selectionChanged.

Problem 3

Bug: If no images are selected, the Previous/Next toolbar buttons remained active according to the 'current'
item as determined by the selection model.
Cause: The decision on whether to enable/disable these buttons did not check whether an item was selected.
Fix: Check selection - if nothing selected, disable both buttons.

Fixes T8123: Context menu and sidebar sometimes populated incorrectly



Test Plan

Go to Browse with a few images in it, enable the Sidebar (F4) and select the Operations tab.
No amount of selecting/deselecting, and right clicking should result in a state where an image is selected but
the operations are greyed out (toolbar) or missing (sidebar and context menu).
In particular, starting with nothing selected, right clicking an image should select that image and display the
full context menu with all operations. The toolbar and Operations tab in the sidebar should also reflect the
correct available operations for that image.

Diff Detail

R260 Gwenview
context-selection (branched from master)
No Linters Available
No Unit Test Coverage
huoni requested review of this revision.Mar 4 2018, 2:00 AM
huoni created this revision.
huoni added inline comments.Mar 4 2018, 2:11 AM

Maybe I shoudn't have included this change. It doesn't actually make any functional difference because if no images are selected, canModify is already false due to the current URL being blank.


Problem 3 fix


After fixing Problem 1, there were duplicate operations when right clicking because deleteLater doesn't delete before the context menu is shown. Since the context menu 'pauses' things, we ended up with two mContainer objects until the menu was closed.
This hides the first one so two aren't visible when the context menu is open.


Problem 2 fix - since we removed slotCurrentChanged, we need to set the URL here.


Problem 1 fix

huoni updated this revision to Diff 30901.Mar 30 2018, 6:51 AM
huoni edited the summary of this revision. (Show Details)
huoni edited the test plan for this revision. (Show Details)
  • Rebase to current master
  • Update test plan
huoni edited the summary of this revision. (Show Details)Mar 30 2018, 6:56 AM
huoni planned changes to this revision.May 25 2018, 10:52 PM

I no longer think getting rid of currentChanged is the right way to go about this, so I plan to have another look at it.

rkflx resigned from this revision.Aug 25 2018, 8:35 AM
huoni abandoned this revision.Sep 16 2018, 12:26 AM

No time to look at this :(