D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham abandoned this revision.
ngraham added a comment.


  Superseded by D20964 , which is the 
correct change.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, abetts
Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, 
alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx resigned from this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, abetts
Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, 
alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham planned changes to this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, 
alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.


  @ngraham I guess you are still working on this, or did you move on to other 
things in the meantime? (And please set the status to "Changes planned", so 
this does not clog up the review queue).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, 
alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


  In D12130#250478 , @ngraham wrote:
  
  > Yes, and I did more investigation myself, and all the glue is in 
plasma-integration. Some changes might be needed there too (or maybe //all// 
the changes will be needed there, we'll see!)
  
  
  Are you sure about the relation to `plasma-integration`? To me it seems that 
the widget can also be used standalone. Also, in KDE4 `plasma-integration` was 
not even there, yet the string was shown in the UI, and different types of 
dialogs were possible. My guess would be that this can be solved entirely in 
`KIO`, perhaps by following code paths relating to `selectMimeTypeFilter`.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, 
alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham added a comment.


  In D12130#250476 , @rkflx wrote:
  
  > In D12130#245921 , @ngraham 
wrote:
  >
  > > I might need some help with this. It seems that the apps whose save 
dialogs show a combobox in place of a filter list are creating a `QDialog` and 
setting the Mime types list with `QDialog::setMimeTypeFilter()`. I haven't yet 
managed to figure out how this call is intercepted and replaced with a 
KFileDialog, or how KFileDialog can detect that there's a combobox of MIME 
types instead of a filter widget.
  >
  >
  > In Gwenview, grep for "Save Image" (has prepopulated combobox) and "Open 
Image" (does not have combobox, but editable and prepopulated filter). The 
difference seems to be the missing `selectMimeTypeFilter` for the latter.
  >
  > Note than in your comment you are referring to `setMimeTypeFilter[s]`, 
which for Gwenview is called in both cases, thus not leading into a code path 
you are interested in.
  >
  > Did not yet have the time (with all the other Diffs to comment on ;) to 
look into how this translates to `KFileDialog`, but does this help already?
  
  
  Yes, and I did more investigation myself, and all the glue is in 
plasma-integration. Some changes might be needed there too (or maybe //all// 
the changes will be needed there, we'll see!)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: safaalfulaij, davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, 
michaelh

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.


  In D12130#245921 , @ngraham wrote:
  
  > I might need some help with this. It seems that the apps whose save dialogs 
show a combobox in place of a filter list are creating a `QDialog` and setting 
the Mime types list with `QDialog::setMimeTypeFilter()`. I haven't yet managed 
to figure out how this call is intercepted and replaced with a KFileDialog, or 
how KFileDialog can detect that there's a combobox of MIME types instead of a 
filter widget.
  
  
  In Gwenview, grep for "Save Image" (has prepopulated combobox) and "Open 
Image" (does not have combobox, but editable and prepopulated filter). The 
difference seems to be the missing `selectMimeTypeFilter` for the latter.
  
  Note than in your comment you are referring to `setMimeTypeFilter[s]`, which 
for Gwenview is called in both cases, thus not leading into a code path you are 
interested in.
  
  Did not yet have the time (with all the other Diffs to comment on ;) to look 
into how this translates to `KFileDialog`, but does this help already?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: safaalfulaij, davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, 
michaelh

D12130: Use the more user-friendly string "File type" in the save dialogs

[Safa Alfulaij]

safaalfulaij added a comment.


  Btw, the `qt` tag is from Qt3 days. I'm not sure if it must be richtext, if 
so then a simple `html` or even KUIT markups are better choice :)
  
  Not sure about string freeze though..

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: safaalfulaij, davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, 
michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.


  > In D12130#245826 , @rkflx wrote:
  > 
  >> > in a new user account
  >>
  >> That's a bit pointless in this case, why would you let your reviewers jump 
through such hoops…
  > 
  > 
  > Not sure what you mean?
  
  Making your reviewers create a new user account while following the test plan 
for a change which does not depend on configuration files seems like a waste of 
time. Better include only relevant steps in the test plan.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham added a comment.


  In D12130#245826 , @rkflx wrote:
  
  > > If necessary I can make the string return to "Filter" for these 
save-with-filter dialogs, but I'm still left wondering what the feature is 
actually for or if Kate is affected by a bug that should be fixed...
  >
  > You absolutely should return to "Filter" for this case (even though I agree 
that for most dialogs it does not make sense), because this is 
application-defined behaviour which you don't know a thing about.
  
  
  I might need some help with this. It seems that the apps whose save dialogs 
show a combobox in place of a filter list are creating a `QDialog` and setting 
the Mime types list with `QDialog::setMimeTypeFilter()`. I haven't yet managed 
to figure out how this call is intercepted and replaced with a KFileDialog, or 
how KFileDialog can detect that there's a combobox of MIME types instead of a 
filter widget.
  
  In D12130#245826 , @rkflx wrote:
  
  > > in a new user account
  >
  > That's a bit pointless in this case, why would you let your reviewers jump 
through such hoops…
  
  
  Not sure what you mean?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham updated this revision to Diff 32093.
ngraham added a comment.


  First fix the casing

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12130?vs=32030&id=32093

BRANCH
  arcpatch-D12130

REVISION DETAIL
  https://phabricator.kde.org/D12130

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.


  Forgot something important:
  
  You should look at Okular's Save As too, where the Automatically select 
filename extension checkbox get's disabled once you deviate from the 
pre-populated file types, i.e. when you type a custom filter. Obviously you 
should not change the text of the label dynamically in this case, so you'll 
have to go with Filter once the widget is editable anyway.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Henrik Fehlauer]

rkflx added a comment.


  > I wonder if having a filter feature even makes any sense when saving files, 
and if it does--why only for Kate?
  
  Well, you should ask #Kate . I suspect 
this is because there cannot be a static list of file types, because there are 
so much weird text-based file formats out there. Say you work on `.xyz` files, 
and want to save `complicated_name_1.xyz` and to save on typing you first 
filter for `.xyz` or even for `complicated_name.xyz` which you then simply 
modify to your needs.
  
  Also, KIO is a library, it can be used in all sorts of applications you don't 
even know about. Maybe there is some weird use case out there where files need 
to be saved to places with thousands of entries where filtering is useful…
  
  > If necessary I can make the string return to "Filter" for these 
save-with-filter dialogs, but I'm still left wondering what the feature is 
actually for or if Kate is affected by a bug that should be fixed...
  
  You absolutely should return to "Filter" for this case (even though I agree 
that for most dialogs it does not make sense), because this is 
application-defined behaviour which you don't know a thing about. In particular 
Kate handles it in such a way that what you type in there is //not// appended 
as the file extension. Your change and your help text imply it is, but actually 
trying it out you'll see that Kate won't append the extension (rightly so, 
because filtering for `*.txt` and typing `aa.txt` as the filename should not 
result in `aa.txt.txt`).
  
  At least we'll now have `setFilterLabel` for customization.
  
  ---
  
  > in a new user account
  
  That's a bit pointless in this case, why would you let your reviewers jump 
through such hoops…

INLINE COMMENTS

> kfilewidget.cpp:1340-1341
>  i18n(autocompletionWhatsThisText);
> +filterWhatsThisText = i18n("This is the file type selector. "
> + "Use it to choose the type for the file that will 
> be saved.");
>  } else if (ops->mode() & KFile::Files) {

Personally I hate it when a help text just repeats what's already written in 
the UI. I'd say here you can utilize `…the format the file will be saved in.`, 
which helps out everyone not understanding "type" in the first place, looking 
for help, and then finding something they recognize.

After all, Wikipedia also calls the concept "File format" (keep "type" for the 
label, though).

> kfilewidget.cpp:1344-1345
> +locationWhatsThisText = "" + i18n("This is the list of files to 
> open. More than "
>"one file can be specified by listing 
> several "
>"files, separated by spaces.") +
>  i18n(autocompletionWhatsThisText);

Indentation?

> kfilewidget.h:362
> + */
> +void setFilterLabel(const QString &text);
> +

Needs `@since` and possibly `@param`.

This addition to the API might also be worth mentioning in the commit message.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham planned changes to this revision.
ngraham added a comment.


  In D12130#245721 , @cfeck wrote:
  
  > In D12130#245680 , @davidc wrote:
  >
  > > "File Type" is a big improvement over "File format."
  > >
  > > Clear, concise, offers consistent capitalization throughout the window, 
and aligns well with the "Name" field above it.
  >
  >
  > https://community.kde.org/KDE_Visual_Design_Group/HIG/Capitalization says 
that we shouldn't use title case for edit box labels.
  
  
  Ah, good point!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Andres Betts]

abetts accepted this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Christoph Feck]

cfeck added a comment.


  In D12130#245680 , @davidc wrote:
  
  > "File Type" is a big improvement over "File format."
  >
  > Clear, concise, offers consistent capitalization throughout the window, and 
aligns well with the "Name" field above it.
  
  
  https://community.kde.org/KDE_Visual_Design_Group/HIG/Capitalization says 
that we shouldn't use title case for edit box labels.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[David C]

davidc added a comment.


  "File Type" is a big improvement over "File format."
  
  Clear, concise, offers consistent capitalization throughout the window, and 
aligns well with the "Name" field above it.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx
Cc: ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12130: Use the more user-friendly string "File type" in the save dialogs

[Nathaniel Graham]

ngraham retitled this revision from "Use the more user-friendly string "File 
format" in the save dialogs" to "Use the more user-friendly string "File type" 
in the save dialogs".
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D12130

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx
Cc: ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham