Bug 306835 - Make selecting directory in directory tree to automatically enter Browse mode
Summary: Make selecting directory in directory tree to automatically enter Browse mode
Status: RESOLVED FIXED
Alias: None
Product: gwenview
Classification: Applications
Component: general (show other bugs)
Version: Other (add details in bug description)
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Gwenview Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-15 12:11 UTC by Alex
Modified: 2018-01-26 07:39 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
STR (370.40 KB, image/jpeg)
2012-09-15 12:14 UTC, Alex
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex 2012-09-15 12:11:32 UTC
Gwenview 2.7.4
KDE 4.7.4

When in View mode, selecting another directory in left side menu automatically opens first file from selected directory in View mode. IMO, this doesn't make any sense. Moreover, sometimes it doesn't open any file at all, showing "No document selected".

Reproducible: Always

Steps to Reproduce:
1. Open any file in View mode
2. Use left side menu to navigate to another directory
Actual Results:  
Either one of these two happens:
* first file in selected directory is opened
* "No document selected" is shown

Expected Results:  
Directory contents should be displayed in Browse mode

See attached picture for additional description
Comment 1 Alex 2012-09-15 12:14:02 UTC
Created attachment 73934 [details]
STR
Comment 2 Valerii Malov 2017-11-09 14:53:24 UTC
Git commit b55420b2ac3dc72ebffdb89dbb9e662d64950ecd by Valeriy Malov.
Committed on 09/11/2017 at 14:52.
Pushed by valeriymalov into branch 'master'.

Try to keep ContextManager in sync with viewed files in MainWindow

Summary:
ContextManager now is responsible for switching to the directory
containing requested URL and selecting it. However, if it is not
possible, URL is still kept (in case of remote URLs), while selection is
cleared (to avoid dragging in local files)

MainWindow now relies on ContextManager's selection and/or
selectedFileItemList instead of ThumbnailView selection. If selection &
currentUrl are empty, refuse to open View tab, otherwise display
selected items.

This should prevent (reduce?) the amount of mismatches between which
files user sees, and which files are being operated upon
(e.g. by FileOpsContextManagerItem)
Related: bug 355493, bug 275807, bug 326190

Test Plan:
Tried playing around to make sure it doesn't break any old behaviour
Tried deleting all image files while in View mode, to make sure we back out when we run out of images
Tried opening an http url and check that operations apply to it unless we select something in browse tab
And then remote image should be unloaded from the View tab since our actions will now affect user-selected items

Tests pass but they don't seem to cover this?

Reviewers: #kde_applications, gateau, rkflx

Reviewed By: gateau, rkflx

Subscribers: ngraham, rkflx, gateau

Differential Revision: https://phabricator.kde.org/D8196

M  +1    -11   app/fileopscontextmanageritem.cpp
M  +27   -70   app/mainwindow.cpp
M  +25   -2    lib/contextmanager.cpp
M  +1    -0    lib/contextmanager.h

https://commits.kde.org/gwenview/b55420b2ac3dc72ebffdb89dbb9e662d64950ecd
Comment 3 null 2018-01-06 12:53:40 UTC
Reopening, because f29f955a164e broke this again.

@Peter: Could you have a look? I suspect your patch was correct, and b55420b2ac3d fixed it only by accident (this bug was only slightly related to this patch anyway), i.e. relying on incorrect behaviour which you fixed.

The actual problem might be in the way passing and receiving the directory URL is done when clicking on an entry in the sidebar. Selecting a directory should never show an image, regardless of whether we select the first image or not.

I'm sure with a bit of patience this whole context/selection cleanup will settle down eventually ;)
Comment 4 Peter Mühlenpfordt 2018-01-08 11:47:11 UTC
The way of passing a new directory url looks ok to me.
The problem is (not) switching to browse mode in MainWindow::openSelectedDocuments(), which is done only if no items are selected.
In cases with just a few images in the new directory, MainWindow::slotDirListerCompleted() triggers too early and the first item is selected - if this is an image, switching to browse mode does not happen.
Selecting a big directory (tested with >5k images) it's ok, since DirLister runs longer.
A solution could be switching to browse mode in MainWindow::slotCurrentDirUrlChanged(). This seems to work in a short test.
Comment 5 null 2018-01-14 21:11:24 UTC
Thanks for looking into it.

Yup, ContextManager::currentDirUrlChanged(const QUrl&) still has the correct URL. Seems like switching to Browse mode was never implemented explicitly for clicking on a folder in the sidebar and this only worked because of b55420b2ac3d's side effect, i.e. an empty selection in MainWindow::openSelectedDocuments().

The fact that it works for slow DirListers is another indicator that currently we rely on unpredictable timing / signal emission behaviour.

One more thing: It would be nice if clicking on the same folder you have open in View mode would switch to Browse mode (currently it does nothing at all, see the early return in MainWindow::openDirUrl).

I couldn't come up with anything better than your suggestion of just triggering the switch manually. Although I wonder whether triggering the action should be done in MainWindow::openDirUrl instead (most of the other actions are triggered in this class). Do you want to submit a Diff or do you want me to do it?
Comment 6 Peter Mühlenpfordt 2018-01-26 07:35:56 UTC
Git commit 8a6a4ea6b9853ef4c9a916d2d91d8a7a02ad6b64 by Peter Mühlenpfordt.
Committed on 26/01/2018 at 07:35.
Pushed by muhlenpfordt into branch 'master'.

Fix issues with clicking directories in tree view

Summary:
Selecting another directory in tree while showing an image in view mode,
opens the first image from the selected directory in view mode. If the
folder contains subfolders, gwenview selects the first one and switches
to browse mode.
This patch changes selection of first **item** back to first **image**
(introduced in D8934) to fix this and toggles between browse/view mode
only after clicking the already selected directory in tree.
Additionally a trailing slash is stripped from the current dir url in
context manager to fix a failing compare to initially stored dir url.

Test Plan:
1. Open image in view mode
2. Click on another directory in tree view
-> Should view the first image in new directory

1. Open image in view mode
2. Click multiple times on the same directory in tree view
-> Should toggle browse/view mode

Reviewers: rkflx

Reviewed By: rkflx

Subscribers: ngraham, rkflx

Differential Revision: https://phabricator.kde.org/D9886

M  +28   -5    app/mainwindow.cpp
M  +2    -0    app/mainwindow.h
M  +2    -1    doc/index.docbook
M  +2    -1    lib/contextmanager.cpp

https://commits.kde.org/gwenview/8a6a4ea6b9853ef4c9a916d2d91d8a7a02ad6b64
Comment 7 Peter Mühlenpfordt 2018-01-26 07:39:34 UTC
According to the Gwenview User Manual, this behaviour is a feature. :-)
See here: https://docs.kde.org/stable5/en/kdegraphics/gwenview/sidebar.html
Therefore we decided to add toggling between View/Browse Mode by clicking multiple times on a folder.