D21907: Show feedback inline when creating new files or folders

[Dominik Haumann]

dhaumann added a comment.


  So it boils down to "let's try this and improve later if necessary" - well 
then :)

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham, filipf, meven
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  In D21907#484197 , @dhaumann wrote:
  
  > These two points were not discussed anymore:
  
  
  I addressed them in https://phabricator.kde.org/D21907#482181:
  
  > 1. Isn't there a better solution by creating the folder and immediately 
select it + change to edit mode?
  
  Not really. The proposed behavior would need to be implemented in every 
single app rather than in KIO, because KIO doesn't know about the view that 
will hold the new file or folder that's being created. Even if we did do add 
view-specifric behavior to every app, then we would still have the problem here 
in KIO's new file/folder dialog, which is app- and view-agnostic. I think it's 
nicer to have it here in KIO, and have every app use the same component, rather 
than re-implement the same set of features in many places for different views.
  
  > 2. Resizing dialogs are usually not preferred.
  
  Yeah, I can understand this concern. In general I find that people get upset 
when the window or the window content changes when a KMessageWidget appears, 
which is a legitimate criticism. However this is a general problem with the 
KMessageWidget's visual design itself, not really with this specific patch. I 
would be in favor of making a "popover" version that shows the message in a 
little pop-up frame that doesn't resize the window or its content. GNOME and 
macOS do this and it's really quite nice.
  
  Of course someone would need to write that component. :)

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham, filipf, meven
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Dominik Haumann]

dhaumann added a comment.


  These two points were not discussed anymore:
  
  1. Isn't there a better solution by creating the folder and immediately 
select it + change to edit mode?
  2. Resizing dialogs are usually not preferred.
  
  If everyone else thinks this change is a good idea - then fine. I sm rather 
sceptical ;)

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham, filipf, meven
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:4fb959b910bf: Show feedback inline when creating new 
files or folders (authored by ngraham).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21907?vs=60299=60329

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #vdg, #frameworks, shubham, filipf, meven
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Méven Car]

meven accepted this revision.
meven added a comment.


  This is look good to me !

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf, meven
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham marked 2 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> meven wrote in knewfilemenu.cpp:1099
> Shouldn't we disable the ok button here ?

No, because this isn't an error condition (it's just a warning).

> meven wrote in knewfilemenu.cpp:1122
> Same here

No, because this isn't an error condition (it's just a warning).

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham updated this revision to Diff 60299.
ngraham marked 4 inline comments as done.
ngraham added a comment.


  Disable Ok button for all error conditions (but not for warning conditions)

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21907?vs=60137=60299

BRANCH
  better-new-file-folder-info (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> knewfilemenu.cpp:1099
> +m_messageWidget->setMessageType(KMessageWidget::Information);
> +m_messageWidget->animatedShow();
> +}

Shouldn't we disable the ok button here ?

> knewfilemenu.cpp:1109
> +m_messageWidget->setMessageType(KMessageWidget::Error);
> +m_messageWidget->animatedShow();
> +}

Shouldn't we disable the ok button here too ?

> knewfilemenu.cpp:1114
> +m_messageWidget->setMessageType(KMessageWidget::Error);
> +m_messageWidget->animatedShow();
> +}

Same here

> knewfilemenu.cpp:1122
> +m_messageWidget->setMessageType(KMessageWidget::Warning);
> +m_messageWidget->animatedShow();
> +}

Same here

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  If there are no formal objections, I'd like to land this.

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham updated this revision to Diff 60137.
ngraham added a comment.
This revision is now accepted and ready to land.


  - Do the right thing on Windows
  - Don't show any warnings when creating filenames with slashes in them (KIO 
currently allows it but substitutes a different character)

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21907?vs=60079=60137

BRANCH
  better-new-file-folder-info (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

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


  Going to better handle Windows and also fix a bug that slipped in.

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  Any more comments?

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Filip Fila]

filipf accepted this revision.
filipf added a comment.
This revision is now accepted and ready to land.


  Looks good from a visual and usability POV.

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

To: ngraham, #vdg, #frameworks, shubham, filipf
Cc: filipf, squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  In D21907#482146 , @dhaumann wrote:
  
  > The much better fix would be: create the folder "New Folder" inline in the 
list view and immediately switch to edit mode, this way you do not need a 
dialog at all but don't loose anything. If the folder already exists, name it 
"New Folder (i)". Yes, that's how it's done in Windows as well, but I think it 
makes a lot of sense.
  >
  > Comments? :)
  
  
  Unfortunately we cannot do this for the dialogs from `KNewFileMenu` because 
KIO has no knowledge of the client's view. This dialog is invoked not only from 
Dolphin, but in the file dialogs and many other places. Dolphin itself could 
implement this as it already has a "rename inline" mode, but it would be a lot 
of extra code. And `KDirOperator` which is used for the file dialogs does not 
have that feature so it would have to gain it first.
  
  In D21907#482146 , @dhaumann wrote:
  
  > Finally, resizing the dialog depending on the error message also looks 
rather hard on my eyes.
  
  
  
  
  In D21907#482169 , 
@squeakypancakes wrote:
  
  > Do the warnings need to below the buttons? I feel like they are kinda out 
of place there.
  
  
  I experimented with putting the warnings elsewhere, but then the problem is 
that they cause items in the user interface below them to jump around as they 
appear and disappear. I know from experience that many people really hate this.

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, LeGast00n, 
michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Squeaky Pancakes]

squeakypancakes added a comment.


  Do the warnings need to below the buttons? I feel like they are kinda out of 
place there.

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: squeakypancakes, dhaumann, aacid, meven, kde-frameworks-devel, LeGast00n, 
michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  The gif depicts a rather extreme case where you're deliberately trying to 
test all the feedback modes in quick succession. :) At @meven's request, I've 
already removed the red error message when the text field is empty.

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: dhaumann, aacid, meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns

D21907: Show feedback inline when creating new files or folders

[Dominik Haumann]

dhaumann added a comment.


  Have not looked at the patch, just the gif: I am rather sceptical if instant 
feedback is a good idea here. First, having an empty line edit will happen a 
lot, usually without the user wanting to press OK. That means the user will 
already get feedback in the middle of the workflow even though he did not yet 
have the intention to click OK yet.
  The same may hold true for the other cases. Too much on-the-fly feedback may 
be annoying and stress the user unnecessarily.
  
  Finally, resizing the dialog depending on the error message also looks rather 
hard on my eyes.
  
  The much better fix would be: create the folder "New Folder" inline in the 
list view and immediately switch to edit mode, this way you do not need a 
dialog at all but don't loose anything. If the folder already exists, name it 
"New Folder (i)". Yes, that's how it's done in Windows as well, but I think it 
makes a lot of sense.
  
  Comments? :)

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: dhaumann, aacid, meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns

D21907: Show feedback inline when creating new files or folders

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> meven wrote in knewfilemenu.cpp:1071
> What about Windows ? maybe use QDir::separator() ?

read the docu for QDir::separator() , you seldom ever want to use it.

https://agateau.com/2015/qdir-separator-considered-harmful/

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: aacid, meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham added a comment.


  In D21907#482087 , @meven wrote:
  
  > It made me think about D17595 .
  >  Would it make sense to upstream this to KIO and unify behavior with 
Open/Save Dialog (it currently has a folder name check using popups).
  >  This is a different matter so it should not stop progress here.
  
  
  This is already in KIO. :)

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Méven Car]

meven added a comment.


  It made me think about D17595 .
  Would it make sense to upstream this to KIO and unify behavior with Open/Save 
Dialog (it currently has a folder name check using popups).
  This is a different matter so it should not stop progress here.

INLINE COMMENTS

> knewfilemenu.cpp:1071
> +if (true /*folders*/) {
> +QStringList folders = text.split(QLatin1Char('/'));
> +if (!folders.isEmpty()) {

What about Windows ? maybe use QDir::separator() ?

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham updated this revision to Diff 60079.
ngraham marked an inline comment as done.
ngraham added a comment.


  Don't show an ugly error when the text field is blank

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21907?vs=60072=60079

BRANCH
  better-new-file-folder-info (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #vdg, #frameworks, shubham
Cc: meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> knewfilemenu.cpp:1047
> +if (text.length() == 0) {
> +m_messageWidget->setText(i18n("Name cannot be blank."));
> +m_messageWidget->setMessageType(KMessageWidget::Error);

I would rather disable the ok button, as long as the user has not entered text, 
rather than using an error.

REPOSITORY
  R241 KIO

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

To: ngraham, #vdg, #frameworks, shubham
Cc: meven, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21907: Show feedback inline when creating new files or folders

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: VDG, Frameworks, shubham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Right now, the new file/folder dialog allows you to give it an invalid name, 
and
  afterwards it will show you a dialog window complaining at you. This is not 
very
  user-friendly, and becomes more user-unfriendly if we want to use it to show 
  warnings rather than errors. Finally it doesn't scale well because as we add 
more
  conditions to check for, we'll need to add more dialog windows.
  
  This patch implements the name feedback/warning/error inline in the original 
name
  dialog itself, and disables the OK button for invalid names.
  
  This patch obsoletes D18384 , D18563 
, and D18599 
.

TEST PLAN
  F6915347: Peek 2019-06-19 17-59.gif 

REPOSITORY
  R241 KIO

BRANCH
  better-new-file-folder-info (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #vdg, #frameworks, shubham
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns