- User Since
- Sep 1 2017, 8:58 PM (20 w, 2 d)
Fri, Jan 19
No problem, thanks for your review! :)
When I do this, a.tmp is renamed to aaa.tmp and b.tmp remains selected. For me, that's the result one would expect.
Wed, Jan 10
Tue, Jan 9
Sun, Jan 7
Nov 19 2017
I think this bug was introduced in Dolphin 2.0(4.8), where he got his new view engine six years ago. Somewhere around f23e9496f303995557b744c03178f5dbd9b35016. Just a guess because i am not able to build this version.
Nov 18 2017
Rename forceRoleEditingFinished() to finishRoleEditing()
Rebase to latest master.
Reverting to previous patch, beacause reacting to KItemListView::scrollOffsetChanged-signal is independent of what/who actual causes the
The problem with catching the wheel event in a different place is that i still need a "connection" between this new place and the KItemListRoleEditor (because the new name is stored in there). So i don't know if that makes it simpler. It would be easier/prettier if we cancle the renaming instead of accepting it, because in that case i don't care about KItemListRoleEditor. But accepting it is the right way to go :)
Unless I find a better solution for it, I'll just go back to my previous approach.
Nov 17 2017
334533 seems to be a tricky one, but definitely related to this Model/View bug, lets see what i can do. I am not very familiar with gwenview as a user, but yes i can have a look at it :)
This patch is pretty simple now. The behaviour is slightly different. Now the renaming gets accepted once you turn the mouse wheel, even if
there is actually nothing to scroll. But i like it that way too :)
Nov 16 2017
The actual problem behind this is an "miscommunication" between View and Model. If you want to rename something, you tell the KItemListView to change the text-role of one of its items by calling KItemListView::editRole(int index, const QByteArray& role). index defines which item to change. But this index is not persistent! Once you scroll through the list, that index may change (because of some performance optimizations in KItemListView::doLayout()i guess). Because of this, the index of an Item in the View differs from the index in the Model(where necessary data like the URL is stored). But by design they must stay the same!
Nov 15 2017
Removed KItemListRoleEditor::encodedFileName() function.
Nov 14 2017
Nov 12 2017
That's just because you click (anywhere, really, not only on the scrollbar), scrolling with the mouse wheel does not trigger renaming as your video shows and unlike Windows.
Yeah you are right.
Windows Explorer is doing it differently: Renaming is confirmed as soon as scrolling starts. On macOS, Finder allows scrolling and confirms once the file disappears from the viewport.
Finder's implementation doesn't sound too bad, but maybe Dolphin's current behaviour of not moving the rename field is intentional? You'd have to ask the Dolphin folks (IRC, kfm-devel list, tag someone on Phab) about that and what they prefer.
To be honest, I also don't know if that is a bug or a feature, but my first thought was, hm there is something broken. :) Ok, have checked that again and Dolphin also renames it as soon as you scroll with the scroll bar.
Nov 10 2017
item and capabilities are const now.
Nov 9 2017
Maybe I should set up phabricator locally on my machine to figure out all its magic :)
Next try :)
Sorry, first time using arc.
Nov 8 2017
I was using this feature the last few days, and I encountered a small bug: there is no check if we have sufficient permission to rename something.
- Go to a directory with no write permission (/usr/ for example)
- Two clicks to rename
- -> inline renaming starts
- -> rename
- error message Access denied to xxxx.
So we should only start two clicks renaming if we are allowed to.
That's rather easy to fix, the only question for me is: should I open a new revision or reopen this one?
Oct 28 2017
I like the reviewing, it gave me proof that you're taking care of kde :) I'm looking forward to supply patches for Dolphin / KIO, so do not hesitate to assign bugs / features to me. Otherwise, I will find some on bugzilla :)
Oct 27 2017
Oct 26 2017
Dragging items cancels twoClicksRenaming now.
Oct 23 2017
Oct 22 2017
If you change focus after selecting an item and then give focus back to dolphin by clicking the selected item it starts the edit. In my book that is not what you want ;)
Oct 21 2017
The focus lost-abort triggers abortTwoClicksRenaming() before the second click. That means that changing focus window does not cancel the rename.
Oct 18 2017
Oct 15 2017
- Better way to abort twoClicksRenaming if the selection of items has changed.
- Use the double-click-interval from Systemsettings instead of the fixed interval of 800ms.
- Fixed some styling.
Oct 13 2017
Changed "fastRenaming" to "twoClicksRenaming".
Oct 12 2017
Then "twoClicksRenaming" it should be :) But since I just came home from Qt World Summit, I will change that tomorrow.
Oh I thought we just leave it like that ^^
Sep 24 2017
Sep 19 2017
Now this feature behaves as follows:
- select an item by single-click, or one is already selected
- wait the "double-click-interval"
- click on the items text ( -> 800ms timer gets started)
- none of the cancellations (see below) happens within that 800ms
- inline-renaming starts
Sep 18 2017
Back from holidays :)
Sep 2 2017
I am currently not able to test it, but by double clicking (within the standard time interval) the text should not trigger fast renaming. If that's currently the case I have to fix my patch.
Thanks for your response! :)