childAtCoordinates() steps back by the layout's spacing until it finds
a widget. With a spacing of 0, which some third-party widgets set, the
point never moves, so dragging something over an empty part of the
panel froze plasmashell.
Always step back by at least 1 px.
BUG: 528228
Prevent saving incorrect icon positions while the desktop is still
setting up its screen geometry.
This fixes the following scenario:
- 'setPerStripe()' is called from QtQuick with a lower 'perStripe'
value (because the desktop geometry is not yet correct) while the
directory is being listed
- directory listing finishes (we skip saving the icon positions)
- 'setPerStripe()' is called again (from QtQuick) with the correct value
This can occasionally happen when turning off one monitor and turning on
another, depending on the hardware.
Behavior change: Saving is skipped when Plasma loads or the desktop
reappears on the screen. There is no need to save when the desktop
loads or reappears.
There is no QWidget to use as parentWidget on this QML path, so
plugins that need an owner for the QActions they return (such as
Dolphin's folder icon action via KFileItemActionPluginV2) got nullptr
and leaked them.
Pass the popup menu through KFileItemActions::setParentObject(), added
in KIO 6.32 (https://invent.kde.org/frameworks/kio/-/merge_requests/2427).
The menu is deleted right after the popup closes, which also frees the
actions. It is not set as parentWidget, since that is also used for job
dialogs that must outlive the popup.
CCBUG: 525162
When something is selected, we pre-generate the drag image so that it's
ready if the user starts a drag. But this is asynchronous, and while
it's generally very fast, a determined with swift fingers can start the
drag before it's ready. There's two major ways that this can happen:
- The user has just moved one or more files, and tries to drag them
again. In this case, they're still selected, the old dragImage is
reused, but as they start from a different position now, the hotspot
adjustment may put it ways away from the pointer.
- The user clicks and drags an icon that was not previously selected.
In this case, there's no image at all, and the icon just vanishes
while dragging.
Both are somewhat challenging to reproduce, but I've found the second
is easiest immediately after plasmashell start. But I've reproduced both
outside of that situation as well.
We do two things to fix this: for the first, we generate the dragImage
not just when the selection changes, but also when icons are moved -
this way it should be ready if the user starts dragging again. Second,
we track on the delegate whether the image is being generated, and skip
starting the drag if it's not. This is fast enough that it should be
ready once the next pixel is reached, so for the user there's no
perceptible difference - effectively just a marginally larger
startDragDistance, but it makes sure that when the drag starts, the icon
does not vanish.
BUG: 527529
Currently, FolderView always opens its context menu at the pointer
position. This makes sense for opening the menu with a right-click, but
the same logic is used when the Menu key is pressed. (If nothing is
selected and we open the containment's context menu instead, the menu
opens at (0,0). This is not how the Menu key works in Dolphin (where it
appears to the bottom right of the current item), somewhat inconvenient
with keyboard navigation (as the pointer position may be far from where
the user is looking) and can be misleading, as it makes it seem like the
menu applies to the file where the context menu is attached to, rather
than the currently selected files.
Instead, we pass in whether the menu should open at the cursor position
or not, and if not, open at the bottom right of the visualParent. This
gives almost identical behavior to Dolphin with regard to where the menu
pops up (with rubberband selection, the two may disagree which is the
currentItem).
BUG: 526282
Ctrl+Shift+N creates a folder on the desktop, but unlike in Dolphin the
"Folder…" entry in the "Create New" menu doesn't show the shortcut.
KNewFileMenu shows the shortcut of the action passed to
setNewFolderShortcutAction(). Dolphin passes its Create Folder action,
but Folder View has none: its shortcuts are handled by the ShortCut
event filter, because actions don't receive key presses in the QML
view. So pass an action that only carries the standard Create Folder
shortcut. It isn't attached to anything, so Ctrl+Shift+N still creates only
one folder.
BUG: 473045
FolderView tracks the pointer position on press, and uses that
information on pointer move to start a drag or rubberband. However,
when something resets the model (such as when a lot of files are deleted
and each deletion resets the model), this information is reset. For the
most part, this means that you can't rubberband while the deletions
happen (you press, the press is reset, you move the mouse and nothing
happens) which isn't great but also not too bad. However, if you manage
to time it just right that the click and move happens fully within the
short period between resets, you'll start the rubberband, then the reset
changes the active rubberband to start at -1, -1 instead of the position
you actually clicked.
A somewhat easier to trigger condition is if some other process removes
or adds files to the desktop while the user is mid-rubberband, such as a
browser downloading things. In that case, the origin will also move from
the clicked position to the top left while the rubberband is active.
This is quite weird. So instead just update the rubberBand's origin if
the rubberband is not active yet. We keep resetting the press state
otherwise to keep the change as small as possible. This means that
starting a rubberband while a large number of files are quickly
added/removed one-by-one will continue to not work unless you get the
timing exactly right. But having to briefly wait while the system is
clearly doing something is less bad than suddenly changing the
rubberband region mid-operation, and changing this can wait for the
new desktop implementation (if feasible then).
BUG: 526073
FIXED-IN: 6.9.0
Only add the "Manual" entry when configuring the containment. The
Descending/Folders first checkboxes now check the selected mode
instead of the combobox index, since index 0 is no longer always
"Manual".
BUG: 513880
preferredWidth() and preferredHeight() already compute a minimum of
one cell (or four rows of the list view) when called with
forMinimumSize = true, but the Layout.minimum* bindings passed
root.inPanel instead. Outside of panels that meant the minimum size
was the full preferred size, so the smaller minimum only ever applied
to the panel popup.
BUG: 515561
Adds a signal so that whenever a file is deleted/moved to trash, we
know to remove it from positioner configuration.
On listing completion, we now get a list of items that actually
exist, so that positioner can remove the extraneous files.
Iterate over all resolutions and skip files that do not exist
in our position array during saving.
BUG: 469445
CCBUG: 507079
FIXED-IN: 6.8.3
On a vertical panel, the size spinbox shown in edit mode for a
non-flexible Panel Spacer cannot go beyond the panel's thickness;
we instead use the panel length regardless of orientation.
BUG: 518799
A replacement FolderModel can register its screen before the previous
model is destroyed. The old model then removes the registration used
by its replacement, causing desktop icons to disappear.
Track registrations by owner and remove the mapping when the last
matching owner releases it. Disconnect mapping signals while changing
a model's folder URL, then refresh its filter after registration.
Add regression tests for overlapping models and final-owner cleanup.
BUG: 527226
FIXED-IN: 6.8.2
Folder View on the desktop forwards unhandled key presses to the
containment, which again forwards it to Krunner. We allow users to
configure this and opt into type-ahead find instead (at least for
Folder View desktops), which handles some key presses and prevents
them from being forwarded. But this is too strict; not only does
the key event need to contain text, it also needs to have no modifiers.
Not handling modified presses is generally reasonable (in case we want
to add shortcuts on the containment level), but shift is also a modifier
and typing a capital letter will interrupt type-ahead find and launch
the krunner window.
Instead, also allow handling (exclusively) shift-modified key events
(still requiring event text). The containment itself will not forward
Ctrl/Alt/Meta-modified events.
m_perStripe is a number of icons per stripe. Positions start at 0, so
the last valid position is m_perStripe -1. So use "less" and "more"
instead of "less/more or equal" comparison.
In practical terms, an icon saved at a position equal to the number of
icons per stripe was treated as fitting even though it was not and
landed on the first slot of the next stripe instead of the first free
slot. This happens when the icons on the desktop are laid in rows and
during startup, a vertical panel causes the number of available icons
per stripe to change by one (the same for horizontal panel and icons
arrangeded columns).
The default "In Columns" actually causes the icons to display in rows and
and "In Rows" actually causes the icons to display in columns. In other
words, their sense is reversed.
The solution is to re-reverse "In Rows" and "In Columns", in the UI, making
"In Rows" effectively the new default setting
BUG: 512993
FIXED-IN: 6.9.0
isUiReady is binded to Plasmoid.containment.corona.isScreenUiReady(root.screen)
but this will be reevaluated only when root.screen changes, not when
corona.screenUiReadyChanged gets emitted.
there was already an explicit enable of the folderview loader
on screenUiReady changed, but assign isUiReady instead, so it's
more robust and the property actually tells the truth
CCBUG:525327
Plasma desktop (FolderModel) previously bypassed the new rename confirmation warnings when renaming items directly on the desktop.
This change replaces the direct setData() call with the shared KIO helper:
KIO::confirmRenameWarning(oldItem, newName, ...)
Now the following warnings are shown consistently everywhere:
- file type/extension change
- hiding file/folder by adding a leading dot
Added renameTargetStillMatches() to guard against race conditions if the item was renamed by another process while the confirmation dialog was open.
When Colors and Plasma Style were set to light and dark themes
respectively, text would not be visible when renaming. So use color from
Kirigami.
BUG: 525319
The Panel edit mode tooltip takes its text from the configure
internalAction. ContainmentItem may add mnemonic markers to the
action text when it's shown in a context menu. This is fine in a
context menu where these are handled, but breaks if it's used directly.
The tooltip tries to strip the marker with regular expressions, but
this broke at some point, and is hard to maintain.
We already have machinery that strips this in Kirigami.MnemonicData.
Let's use it here as well for simpler and more robust code.
BUG: 488578
The isPopup property used throughout FolderView is confusingly named:
there are several things it could refer to, in particular the popup
preview dialog you get from hovering over a folder (or pressing the
corresponding button on the desktop), or the panel popup you get when
clicking a compactRepresentation in a panel. It does not refer to any
of these; instead it's true if the plasmoid type is not Floating. This
is the case when the FolderView is in a panel, whether as a popup or not
(e.g. a folder view in a very wide side panel might have the fullRep in
the panel directly with no popup, but isPopup would still be true.
Instead rename it to inPanel, which is what we're actually testing for.
RenameEditor tries to do an unqualified lookup for the PlasmoidItem to
get its useListViewMode property, but this fails because root is also
the id of the base of RenameEditor, so it does a qualified lookup there
instead (and fails because no such property exists). As a consequence,
the list view mode editor behaves like the grid view mode editor (and
e.g. wraps the text onto multiple lines when only one is visible).
Instead pass in useListViewMode from outside (where the unqualified
lookup succeeds). We also do the same for isPopup - the completely
unqualified lookup succeeds here, but we might as well do this now
to make the linter happy and prevent future problems.
The updateContextualActions function uses a variable that's not
declared, so it becomes an unqualified lookup that fails (as there is
no such property anywhere).
Instead, declare it as a const local variable.
FolderViewDialog is supposed to autoclose when neither it nor a child
dialog contains the pointer anymore (after a quite generous humanMoment
delay). This doesn't work for two reasons: trying to get the FolderView
containsMouse property fails because it doesn't have one (only the inner
MouseEventListener does, but this is not aliased in FolderView). And
even if that would work, closing the dialog fails because it tries to
destroy the dialog with the wrong id.
So we alias that property in FolderViewDialog, and use the correct id.
Along the way we strengthen some types to make them clearer to tooling
and assert them where necessary. Along the way we also make some
properties that should not be changed read-only.
The plugin has a regularly defined logging category in the CMakeLists
file, but FolderModel creates its own category outside CMake. This
should also use CMake, but the category is used exactly once, so doesn't
seem particularly useful in the first place, and someone configuring the
folder plugin debug categories likely expects this to apply to
FolderModel logs.
Just use the regular Folder plugin debug category for the one
categorized log message.
Most of the Folder backend uses declarative type registration, but the
KRunnerChecker does not. This is inconsistent, adds more integration
code, and causes linter warnings.
Instead, use declarative type registration here as well.
FolderView sets the dragX and dragY properties immediately before
starting the drag, and unsets them immediately after. They are only
ever used when dropping (either in the dropHandler, or when the
backend sends the move signal). At that point both are always unset
to -1 already, so the code does nothing except make the logic more
complicated.
We can just remove them. The only thing they seem to plausibly do is
to skip getting the move machinery involved in case we're dropping an
icon on itself, but this doesn't work (and likely hasn't for a very long
time). If we want to actually do that, it would seem to be much simpler
to store a reference to the dragged item and compare against that
instead of the cursor position that we then use to look up what the item
was later.
FolderView keeps track of the index of the last item that was the
currentItem before the current one. This requires a lot of state
tracking throughout the component, and is only used for one thing:
when determining whether to have a click on the label initiate a
rename, to see if the item was already selected before we pressed it.
We can do this in a much more local way, by just checking whether the
pressed item is selected when pressing it. (Clicking it presses it first
so we don't need to care about what the state is outside that). We also
implement a slight behavior change; before, if multiple files were
selected, a label click would sometimes initiate a rename but mostly
not, depending on the specific order the currentIndex was set in (which
is not easy to figure out). Dolphin seems to use a much simpler pattern:
it only works if one file is selected. We implement the same here, but
as this information is not currently available in the frontend, we add
a property that reports the count of selected items.
We can use Application.styleHints instead of Qt.styleHints; this version
is available to tooling. While we're at it, we switch from assigning the
doubleClickTimer interval imperatively to a binding; this is never
changed so there's no reason to do it imperatively.
FolderItemActionButton has a MouseArea to handle clicking and hovering.
This can just be a pair of TapHandler and HoverHandler, which is more
modern QML style.
The tooltip contents are currently set imperatively on hover. They are
required properties so are retrieved anyway, and setting them should be
cheap enough that we can simplify this to a declarative binding.
We define handers for FolderModel signals three different ways: by
directly binding to the handler, by imperatively connecting to the
signal in Component.onCompleted, and with a Connections object. These
are all in the same file, which is also where the FolderModel
itself is instantiated. There does not seem to be a good reason not to
just use the first one for all of them, as it's the simplest and this
will keep all the handlers in one place.
BackButtonItem shares a DropArea with FolderView. This means that the
FolderView drag handling needs to handle both, which makes the logic
unwieldy and more complicated than it needs to be, as both are otherwise
completely separate.
Instead give BackButtonItem its own DropArea.
BackButtonInterface contains implementation logic specific to FolderView
and accesses its functions and properties through unqualified lookups.
This is bad for separation of concerns and tooling, and makes the logic
hard to follow.
Instead, define a clear interface in terms of properties and signals,
and clearly split the parts that are internal to BackButtonItem and
FolderView respectively. This also resolves all current linter warnings,
except for one unqualified lookup of a constant property defined in the
PlasmoidItem.
BackButtonItem activates on click with left click, but on press when
using a dedicated mouse Back button. To do so, it uses a property
to store whether the click should be ignored (if it already actived on
press).
This is just unnecessary state tracking; we can instead test whether the
click was done with the Back button. (In principle, the existing
solution could work to not do double-back if the user presses both
buttons at the same time, but people don't do that, and in practice it
doesn't really work anyway - you're still likely to double-back if you
actually try it on the existing implementation.)
We check each time before using it whether we can actually go back. In
some cases this is unnecessary (the back button can only ever be visible
if we can), is more complicated than it needs to be (useListView depends
on isPopup, so testing both is unnecessary) and it duplicates the logic.
Instead we move the check into doPopup, and as we may want to not accept
events if we can't actually go back (mostly out of principle, I don't
think anything depends on this), we have it return whether we were able
to. Along the way, we are more consistent about rejecting the event if
not, and eliminate two slightly related instances of unnecessary isPopup
checks.
For list mode FolderViews, we have a back button up top. We only need
this sometimes, so we dynamically create and destroy it. The problem
is that we do this imperatively, which is a fair bit of state tracking.
We can do the same thing declaratively with a Loader, eliminating the
state tracking and allowing us to make another property readonly. The
only small downside is that we check whether we're dragging something
onto the back button using childAt, and this is now a little more
complicated with the Loader.
