[FileWidget] Replace "Filter:" with "File type:" when saving with a limited list of mimetypes
ClosedPublic

Authored by ngraham on May 2 2019, 2:47 PM.

Details

Summary

The Filter combobox currently always has the label "Filter:" This is appropriate in most
cases, but becomes inaccurate when saving with a limited list of mimetypes. In this case,
the combobox loses its text field and becomes a file type chooser rather than a filter.

This patch changes the label to "File type:" for that use case.

BUG: 79903
FIXED-IN: 5.59

Test Plan

Opening a file: no changes

Saving a file with no mimetype filter: no changes

Saving a file with a limited list of mimetypes: label is changed

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.May 2 2019, 2:47 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 2 2019, 2:47 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.May 2 2019, 2:47 PM
apol added a subscriber: apol.May 2 2019, 7:11 PM
apol added inline comments.
src/filewidgets/kfilewidget.cpp
739

Isn't it a bit weird to do this in setMimeFilter?

ngraham added inline comments.May 2 2019, 7:29 PM
src/filewidgets/kfilewidget.cpp
739

I did it in setMimeFilter since that's where it's determined whether or not there's a limited assortment of mimetypes, which is what controls what the string should be. Conditionalizing it based on mode seemed safe enough considering that there's mode-specific code a few lines above, on 741.

apol added inline comments.May 3 2019, 3:28 PM
src/filewidgets/kfilewidget.cpp
739

::setOperationMode would need to update that as well, no? Then it would need splitting into a separate function, otherwise
setOperationMode(Saving); setOperationMode(Opening) will have the wrong behaviour.

ngraham updated this revision to Diff 57492.May 3 2019, 5:55 PM
ngraham marked 3 inline comments as done.

Put it in a function and then call that function in all the places where things might change

ngraham updated this revision to Diff 57493.May 3 2019, 5:57 PM

Revert unintentional change to KFileWidget::currentMimeFilter()

apol added a comment.May 3 2019, 11:31 PM

+1 looks good to me besides the nitpick.

src/filewidgets/kfilewidget.cpp
594

Pass this as parent, I guess when it's added to the layouts somehow it will get reparented, but I'd say it's a good practice.

ngraham updated this revision to Diff 57516.May 4 2019, 3:02 AM
ngraham marked an inline comment as done.

Don't forget to set the parent

ngraham updated this revision to Diff 57517.May 4 2019, 3:03 AM

Fix typo in comment

elvisangelaccio accepted this revision.May 5 2019, 6:12 PM

Please wait the tagging before pushing.

This revision is now accepted and ready to land.May 5 2019, 6:12 PM

Yep, will do. Thanks!

pino added a subscriber: pino.May 5 2019, 6:45 PM
pino added inline comments.
src/filewidgets/kfilewidget.cpp
2804–2806

why is this limited to the saving mode? you can perfectly use a mimetype filter when opening files

ngraham added inline comments.May 5 2019, 6:48 PM
src/filewidgets/kfilewidget.cpp
2804–2806

When opening, the mimetype chooser still has filtering functionality and isn't a drop-down menu combobox like it is when saving. I couldn't come up with an adequate string to communicate both purposes and figured "Filter" was good enough.

This revision was automatically updated to reflect the committed changes.