| rkflx |
| Gwenview |
The paste action is disabled on Gwenview start. This is caused by
initializing the action state before setting the current directory
url in ContextManager. The state updates only on clipboard changes
and does not reflect the current permissions to paste into a folder.
This patch updates the paste action state on changing the directory
and the selection, which both affect the paste destination.
BUG: 276255
FIXED-IN: 18.04.2
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| app/fileopscontextmanageritem.cpp | ||
|---|---|---|
| 218 | This first update can be omitted at the moment, but I think we should keep it if the order of setting up ContextManager and FileOpsContextManagerItem changes sometime. | |
Patch works as advertised.
One issue though - in the case of a read-only directory, this fact isn't very obvious. Before, there was an error message which made it explicit, but simply a disabled action might be confusing and appear to be a bug if the user doesn't realise.
One solution is to put "(read only)" or something similar in the titlebar or status bar, but since Gwenview is primarily a viewer not an editor, I'm not sure this is a good solution.
Thoughts?
You're right - the de-/activation of the paste action is a bit strange. ATM it depends on the current directory but is updated only on clipboard changes.
I think we have two options to make it consistent:
The first option sounds a bit more handy to me.
Agreed, I think this is one of those cases where an error message is better than disabling the action/button/menu item.
Hm, seems like the original intention of this change (0861dd248c01) was to adapt to the Dolphin behaviour which in fact disables the paste action for non writable folders.
In that case it's probably better to have consistency and copy Dolphin.
However, I notice that in Dolphin, pressing the shortcut (Ctrl+V) still results in an error dialog. So it's not the entire action that is disabled, just the menu item. We should copy that behaviour I think.
Strange - I can't reproduce this. Tried with current master and installed version 17.12.3.
I don't think this is possible with the standard QAction (if it's not a bug).
How did you provoke this error dialog?
Update state when current directory or selection changes
| app/fileopscontextmanageritem.cpp | ||
|---|---|---|
| 218 | It's called from updateActions() now, so not needed anymore. | |
Seems like you already considered everything which was worth discussing, and I agree with your decision to be consistent with Dolphin. In general disabling options seems like the way to go, e.g. in Okular the print action is disabled once there is no document open instead of warning the user that Ctrl+P won't work in that case.
Patch LGTM, thanks! (Although there seem to be some redundant/duplicate calls to updateActions in some situations – nothing for this patch, though…)