The de-/select (-/+) hover button of the thumbnails in Browse
Mode or thumbnail bar in View Mode does not reflect the actual
selection state sometimes.
This patch updates the button state when the selection changed.
BUG: 394406
FIXED-IN: 18.04.2
huoni |
Gwenview |
The de-/select (-/+) hover button of the thumbnails in Browse
Mode or thumbnail bar in View Mode does not reflect the actual
selection state sometimes.
This patch updates the button state when the selection changed.
BUG: 394406
FIXED-IN: 18.04.2
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
(Not in the scope of this fix - I just noticed that PreviewItemDelegate and ThumbnailBarItemDelegate duplicates some code but it seems not that easy to merge.)
lib/thumbnailview/previewitemdelegate.cpp | ||
---|---|---|
886 | Not needed here (and the other place) anymore, since the select() above leads to the call. | |
lib/thumbnailview/thumbnailbarview.cpp | ||
516 | The selectionChangedSignal is emitted only in the direct base class in ThumbnailView::selectionChanged(). |
@huoni Here's how I would start with assessing whether the patch works correctly (apply before and after arc patch and compare output on startup and when testing both keyboard and mouse navigation):
Sometimes it's also helpful to monitor signals in GammaRay (not sure if applicable here).
If I see this correctly, the only time the buttons weren't updated was when the hover did not change, but the selection did. E.g. when Ctrl + clicking, or ⇧ + arrow. Oops, just realised this was cleared up in the Test Plan.
If so, then the patch LGTM.
That said, two very minor points:
Thanks @rflx for the debug diff. I don't think the above is too important, it's probably fine as is - there aren't many unnecessary calls, and I'd guess it's an inexpensive operation.
There are two different instances of ThumbnailBarView created in ViewMainPage and FullScreenContent, so the slot is called for both thumbnail bars.
- Changing the bar selection in View mode seemingly triggers the Browse hover buttons to update, which is unnecessary
The selection events are sent to visible and not visible thumbnail view/bars (with or without this patch). Maybe we could prevent this, but then have to update the selection on mode change. I think this could get tricky.
Ah, forgot about the fullscreen bar. Yeah I don't think it's worth filtering events, it's probably just unnecessary complication. Sorry forgot to accept the patch earlier.