Implement a custom AddDialog
ClosedPublic

Authored by rthomsen on Jul 8 2016, 3:43 PM.

Details

Reviewers
elvisangelaccio
Summary

This diff implements an AddDialog class that is used instead of QFileDialogs for adding files/folders to an archive. Now both folders and files are added to an archive with a single action in Part, instead of the two separate actions (Add File/Add Folder).

AddDialog has a button which opens a new dialog that allows setting advanced compression settings (currently only compression level). Since compression options are used both when creating a new archive and adding files to an existing archive, a new class called CompressionOptionsWidget was created which is used by both AddDialog and CreateDialog.

Test Plan
  • Add file(s) to an archive.
  • Add folder(s) to an archive.
  • Add both files(s) and folder(s) simultaneously.
  • Try customizing compression level.

Diff Detail

Repository
R36 Ark
Lint
Lint Skipped
Unit
Unit Tests Skipped
rthomsen updated this revision to Diff 5023.Jul 8 2016, 3:43 PM
rthomsen retitled this revision from to Implement a custom AddDialog.
rthomsen updated this object.
rthomsen edited the test plan for this revision. (Show Details)
rthomsen added a reviewer: elvisangelaccio.
rthomsen set the repository for this revision to R36 Ark.
rthomsen added a project: Ark.
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptJul 8 2016, 3:43 PM

The dialog works fine, I only have minor comments!

autotests/kerfuffle/CMakeLists.txt
17

Think links all the executables to KioFileWidgets. This could result in a slower build.
Please add a dedicated ecm_add_test() below for this new test.

kerfuffle/adddialog.cpp
62

Maybe this button could have the "option" icon? (the one used also in the Settings menu)

71

i18nc("@action:button", "Add")

104

Is this really necessary?

121

optionsWidget is a child of optionsDialog, no need to delete it manually.

kerfuffle/adddialog.h
65

checked is not used, we can drop it (the connect() will work fine).

kerfuffle/compressionoptionswidget.h
48

Q_NULLPTR instead of 0

kerfuffle/createdialog.cpp
96–97

Can't you add this widget in createdialog.ui? You can call setMimeType() instead of passing the mimetype to the constructor.

part/part.cpp
1244

The name of the actions and the title of the dialog are not consistent:

  • Action in the menu => Add File...
  • Action in the toolbar => Add File
  • Title of the dialog => Add Files

Not sure what should we do, though...

rthomsen updated this revision to Diff 5028.Jul 8 2016, 8:44 PM
rthomsen edited edge metadata.
  • Implement Elvis' suggestions.
  • Some minor bug fixes.
  • The libarchiveplugin stuff was committed separately.
rthomsen updated this revision to Diff 5029.Jul 8 2016, 8:51 PM

Move CompressionOptionsWidget to the right layout in CreateDialog.ui.

rthomsen marked 9 inline comments as done.Jul 8 2016, 8:53 PM
rthomsen added inline comments.
kerfuffle/adddialog.cpp
104

No

ltoscano added inline comments.
part/part.cpp
1244

If the result of the action (through a dialog in this case) allows to add multiple files, then it should be Add Files, yes - but no ellipsis in the dialog title, only in the menu.

rthomsen marked an inline comment as done.Jul 8 2016, 9:27 PM
rthomsen added inline comments.
part/part.cpp
1244

Ok, so the menu item and toolbar item should have ellipsis, but not the dialog title?

ltoscano added inline comments.Jul 8 2016, 9:37 PM
part/part.cpp
1244

Yes:
"Indicate a function that needs additional information (including a confirmation) by adding an ellipsis at the end of the label (e.g. Save as…)."
(this is not only for the KDE world afaik)
https://techbase.kde.org/Projects/Usability/HIG/Menu_Bar

rthomsen updated this revision to Diff 5032.Jul 8 2016, 10:10 PM
rthomsen updated this object.

Add ellipsis to text of the "Add Files" action.

rthomsen updated this object.Jul 8 2016, 10:13 PM
rthomsen marked 3 inline comments as done.Jul 8 2016, 10:25 PM
rthomsen updated this revision to Diff 5041.Jul 9 2016, 4:22 PM

Remember last-used directory when adding files.

rthomsen updated this revision to Diff 5046.Jul 9 2016, 5:39 PM

Improve unittest so that it tests that passing a compression level works and that passing no compression level results in the default being used.

rthomsen updated this revision to Diff 5211.Jul 15 2016, 4:36 PM

Synced with current master.

elvisangelaccio accepted this revision.Jul 16 2016, 10:19 AM
elvisangelaccio edited edge metadata.

Good to go now!

This revision is now accepted and ready to land.Jul 16 2016, 10:19 AM
rthomsen closed this revision.Jul 17 2016, 8:45 AM

Committed with rARK07269ec72e0f.