Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple selection does not work for directories. #1869

Open
m3nu opened this issue Dec 11, 2023 Discussed in #1867 · 17 comments · May be fixed by #1938 or #1871
Open

Multiple selection does not work for directories. #1869

m3nu opened this issue Dec 11, 2023 Discussed in #1867 · 17 comments · May be fixed by #1938 or #1871
Labels
good first issue Simple change to start learning code base type:bug Something doesn't work as intended

Comments

@m3nu
Copy link
Contributor

m3nu commented Dec 11, 2023

Discussed in #1867

Originally posted by user723045 December 10, 2023
Hello.
On the Source tab, I add directories and files to the list for backup.
I noticed that when I select multiple files by holding down the shift key it works fine.
But when I try to add several directories at a time in the same way, it does not work and only one single directory is added.
This is very inconvenient if you need to select many directories, you have to make many more clicks.

I'm using borg 1.2.6 and Vorta 0.8.12 on Kali linux.
I installed packages using apt.

@m3nu m3nu added the type:bug Something doesn't work as intended label Dec 11, 2023
@m3nu
Copy link
Contributor Author

m3nu commented Dec 11, 2023

Possible bug in how we deal with dirs/files.

@m3nu m3nu added the good first issue Simple change to start learning code base label Dec 11, 2023
@SAMAD101
Copy link
Collaborator

SAMAD101 commented Dec 14, 2023

In src/vorta/utils.py:

def choose_file_dialog(parent, title, want_folder=True):
    dialog = QFileDialog(parent, title, os.path.expanduser('~'))
    dialog.setFileMode(QFileDialog.FileMode.Directory if want_folder else QFileDialog.FileMode.ExistingFiles)
    dialog.setParent(parent, QtCore.Qt.WindowType.Sheet)
    if want_folder:
        dialog.setOption(QFileDialog.Option.ShowDirsOnly)
    return dialog

This is the function responsible for selecting sources, as we are using QFileDialog which doesn't have any feature to allow us to select multiple directories in one go. To do this, we need to find an alternative.

@real-yfprojects
Copy link
Collaborator

Is this really a limitation of Qt or are we using QFileDialog incorrectly?

@SAMAD101
Copy link
Collaborator

Is this really a limitation of Qt or are we using QFileDialog incorrectly?
Its is a limitation. Though alternatively, we can use QTreeView with QAbstractItemView.SelectionMode.MultiSelection selection mode.

def choose_file_dialog(parent, title, want_folder=True):
    dialog = QFileDialog(parent, title, os.path.expanduser('~'))
    dialog.setFileMode(QFileDialog.FileMode.Directory if want_folder else QFileDialog.FileMode.ExistingFiles)
    dialog.setParent(parent, QtCore.Qt.WindowType.Sheet)
    if want_folder:
        dialog.setOption(QFileDialog.Option.ShowDirsOnly)
        dir_dialog = dialog.findChild(QTreeView)
        dir_dialog.setSelectionMode(QAbstractItemView.SelectionMode.MultiSelection)
    return dialog

But here, just the problem is that dialog.selectedFiles() will not return the directories so, need to figure out a way to make it happen somehow.

@SAMAD101 SAMAD101 linked a pull request Dec 14, 2023 that will close this issue
9 tasks
@SAMAD101
Copy link
Collaborator

Made a PR, not complete yet, now you can select multiple directories but none of them get added to the sources if selected multiple. If you select only one, that will do. So, it's halfway there.

@madeddy
Copy link

madeddy commented Jan 16, 2024

Is this really a limitation of Qt or are we using QFileDialog incorrectly?

Its is a limitation. Though alternatively, we can use QTreeView with QAbstractItemView.SelectionMode.MultiSelection selection mode.

I think QFileDialog is better suited for single file/dir selection such as save dialog, than is the case here. Even if the manual says it's for single and multi.

I agree with the use of a treeview way as stated. Its more efficient and user friendly IMHO. I've worked with such a GUI in KDE's integrated kup-backup solution and it was very good.

Example for treeview:
kup_treeview

The checkboxes are great for this i think.
If you guys want to take a look at the code they use... warning, its in C++ / QT). This should be the right func for this:filedigger.cpp#L122

Greets

@rajb957
Copy link

rajb957 commented Jan 31, 2024

Is it still open? Can I work on this? I have worked on qt5 before, I think i can fix this. Can you assign it to me?

@SAMAD101
Copy link
Collaborator

Is it still open? Can I work on this? I have worked on qt5 before, I think i can fix this. Can you assign it to me?

I'm still looking into it, but you can surely help

@real-yfprojects
Copy link
Collaborator

Is it still open? Can I work on this? I have worked on qt5 before, I think i can fix this. Can you assign it to me?

Whoever provides a mergeable PR first will be merged. However you can also choose to tackle another issue as there are plenty floating around.

@rajb957
Copy link

rajb957 commented Feb 14, 2024

I've analyzed the issue and concluded that it can be effectively resolved by using a custom QFileDialog. Most native file dialogs do not support selecting multiple directories due to inherent limitations in their standard interfaces. However, a custom file dialog can overcome this problem.

By enabling the DontUseNativeDialog option, QFileDialog switches from using the operating system's native dialog to a Qt-implemented dialog. This custom dialog is consistent across all platforms and can be extensively customized. Importantly, it supports features not available in native dialogs, such as the ability to select multiple directories at once. This functionality is essential for our use case but is typically unsupported by the default file dialogs on most operating systems.

Should I proceed with implementing this solution? It would involve using a custom QFileDialog to open when the user needs to select multiple folders, allowing for the selection of multiple directories in a single action.

@real-yfprojects
Copy link
Collaborator

It would involve using a custom QFileDialog

You mean custom in the sense of not being native, although provided by the framework? In that case this seems to be a good solution. If this would mean implementing a file dialog in Vorta's code base, I would oppose the idea.

@rajb957
Copy link

rajb957 commented Feb 14, 2024

Yes i mean custom in sense of not being native, but provided by the framework. It would uniformize the FileDialog across all operating systems and the FileDialog set by the user's file manager will not be used.

@rajb957 rajb957 linked a pull request Feb 14, 2024 that will close this issue
9 tasks
@rajb957
Copy link

rajb957 commented Feb 14, 2024

I have made a pull request and allowed multiselecting directories at other places as well including archives and choosing borg backup directory at places where only one directory needs to be selected if multiple directories are selected the first directory selected will be taken

@rajb957
Copy link

rajb957 commented Feb 15, 2024

Hello, I've made some updates to the pull request. I believe the changes are ready for review and merging. Could you please take a look and let me know if there are any further changes needed or if it's ready for merging? Thanks!

@rajb957
Copy link

rajb957 commented Feb 16, 2024

Hi there, I've just noticed there might be one trailing whitespace that I forgot to remove. However, aside from that, all test cases pass on my local machine. I'm a bit puzzled by the font-related test case failure. Could you please take a look at the latest changes and help identify the issue with the font test case? Your insights would be greatly appreciated. Thanks in advance for your assistance!

@hasrat17
Copy link

Hi @real-yfprojects, If this issue is still open, I would like to work on it. Can you assign it to me?

@m3nu
Copy link
Contributor Author

m3nu commented Feb 25, 2024

We already have 2 PRs for this. Would be good to take the best of each and just merge something

https://github.com/borgbase/vorta/pulls?q=is%3Apr+is%3Aopen+multiselect+OR+multiselection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Simple change to start learning code base type:bug Something doesn't work as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants