feat: added drag and drop when selecting iso #920
feat: added drag and drop when selecting iso #920gastoner wants to merge 2 commits intoFedoraQt:mainfrom
Conversation
Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
There was a problem hiding this comment.
Pull request overview
This pull request adds drag and drop functionality for ISO file selection, enhancing the user experience by allowing users to drag image files directly onto the application window instead of only using a file dialog.
Key changes:
- Replaced the previous RowLayout (containing a label and button) with a Rectangle containing integrated drag and drop support
- Added visual feedback during drag operations with color changes and border styling
- Maintained backward compatibility with click-to-browse functionality through an overlaid MouseArea
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| QQC2.Label { | ||
| text: releases.localFile.iso ? (String)(releases.localFile.iso).split("/").slice(-1)[0] : ("<font color=\"gray\">" + qsTr("None") + "</font>") | ||
| Layout.fillWidth: true | ||
| elide: QQC2.Label.ElideRight | ||
| anchors.centerIn: parent | ||
| horizontalAlignment: Text.AlignHCenter | ||
| text: { | ||
| if (releases.localFile.iso) | ||
| return (String)(releases.localFile.iso).split("/").slice(-1)[0] + "\n" + qsTr("Click to change") | ||
| else if (dropArea.containsDrag) | ||
| return qsTr("Drop to select") | ||
| else | ||
| return qsTr("Drop image file here or click to browse") | ||
| } | ||
| opacity: releases.localFile.iso ? 1.0 : 0.6 | ||
| } |
There was a problem hiding this comment.
The multi-line text in the label (filename + "Click to change") may not wrap properly or could be truncated on narrow screens. The label doesn't have explicit wrapMode or elide properties set, which could result in text overflow. Consider adding "wrapMode: Text.WordWrap" to the Label to ensure the text displays correctly across different screen sizes.
There was a problem hiding this comment.
I think this should have https://doc.qt.io/qt-6/qml-qtquick-text.html#lineCount-prop and https://doc.qt.io/qt-6/qml-qtquick-text.html#wrapMode-prop set as suggested. I think having maximum of two lines should be perfectly fine.
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } |
There was a problem hiding this comment.
The drop handler doesn't provide user feedback on the drop action's success or failure. When a file is dropped, there's no visual indication if the operation succeeded or if there was an error (e.g., file doesn't exist, wrong format). Consider adding some form of feedback mechanism, such as a temporary status message or visual indicator, to inform users whether their drop action was successful.
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } |
There was a problem hiding this comment.
The dropped file is not validated before being passed to selectLocalFile. Unlike the FileDialog which has nameFilters to restrict file types, the DropArea accepts any URL. This could lead to confusing error messages when users drop unsupported file types. Consider adding validation to check if the file extension matches the expected formats (.iso, .raw, .xz) before calling selectLocalFile, and potentially provide user feedback when an invalid file is dropped.
There was a problem hiding this comment.
Should there be some validation that the URL (selected file) is an ISO or any other supported format?
| MouseArea { | ||
| anchors.fill: parent | ||
| cursorShape: Qt.PointingHandCursor | ||
| onClicked: { | ||
| if (portalFileDialog.isAvailable) | ||
| portalFileDialog.open() | ||
| else | ||
| fileDialog.open() | ||
| } | ||
|
|
||
| Connections { | ||
| target: portalFileDialog | ||
| function onFileSelected(fileName) { | ||
| releases.selectLocalFile(fileName) | ||
| } |
There was a problem hiding this comment.
The MouseArea and DropArea both fill the entire parent rectangle, which can lead to event handling conflicts. When a user drags a file over the area, both the MouseArea cursor change and DropArea drag detection are active simultaneously. While this may work in practice due to event propagation, it's clearer to disable the MouseArea during drag operations. Consider setting "enabled: !dropArea.containsDrag" on the MouseArea to prevent potential interaction issues.
| DropArea { | ||
| id: dropArea | ||
| anchors.fill: parent | ||
|
|
||
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } | ||
|
|
||
| FileDialog { | ||
| id: fileDialog | ||
| nameFilters: [ qsTr("Image files") + " (*.iso *.raw *.xz)", qsTr("All files (*)")] | ||
| onAccepted: { | ||
| releases.selectLocalFile(currentFile) | ||
| } | ||
| } |
There was a problem hiding this comment.
The DropArea lacks keyboard accessibility. Users who navigate via keyboard cannot drop files using this interface, and the MouseArea doesn't handle keyboard events like Enter/Space to trigger the file dialog. Consider adding a FocusScope or making the MouseArea respond to keyboard activation events to ensure the file selection functionality is accessible to keyboard-only users.
grulja
left a comment
There was a problem hiding this comment.
This also shows a bug that is reproducible regardless of this change. The [Write] button in case we are about to write selected ISO shouldn't be enabled until an USB drive is selected or until an ISO is selected.
src/app/qml/DrivePage.qml
Outdated
| Layout.fillWidth: true | ||
| Layout.preferredHeight: units.gridUnit * 5 | ||
|
|
||
| color: dropArea.containsDrag ? Qt.rgba(0.2, 0.5, 0.8, 0.1) : "transparent" |
There was a problem hiding this comment.
I don't think any color should be hardcoded here. Isn't there a color from palette that we can use instead?
src/app/qml/DrivePage.qml
Outdated
| RowLayout { | ||
| id: fileCol | ||
|
|
||
| // Simple drop area rectangle |
Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| var url = drop.urls[0].toString().toLowerCase() | ||
| if (url.endsWith(".iso") || url.endsWith(".raw") || url.endsWith(".xz")) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The DropArea should provide feedback to the user when a file is rejected (e.g., wrong file type). Currently, when an unsupported file type is dragged and dropped, the user receives no feedback explaining why nothing happened. Consider adding visual feedback or a message when an invalid file type is dropped.
| MouseArea { | ||
| anchors.fill: parent | ||
| cursorShape: Qt.PointingHandCursor | ||
| enabled: !dropArea.containsDrag | ||
| onClicked: { | ||
| if (portalFileDialog.isAvailable) | ||
| portalFileDialog.open() | ||
| else | ||
| fileDialog.open() | ||
| } | ||
|
|
||
| Connections { | ||
| target: portalFileDialog | ||
| function onFileSelected(fileName) { | ||
| releases.selectLocalFile(fileName) | ||
| } | ||
|
|
||
| DropArea { | ||
| id: dropArea | ||
| anchors.fill: parent | ||
|
|
||
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| var url = drop.urls[0].toString().toLowerCase() | ||
| if (url.endsWith(".iso") || url.endsWith(".raw") || url.endsWith(".xz")) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| FileDialog { | ||
| id: fileDialog | ||
| nameFilters: [ qsTr("Image files") + " (*.iso *.raw *.xz)", qsTr("All files (*)")] | ||
| onAccepted: { | ||
| releases.selectLocalFile(currentFile) | ||
| } | ||
| } |
There was a problem hiding this comment.
The MouseArea and DropArea are both set to fill the parent, which can cause input handling conflicts. When both components occupy the same space, there may be undefined behavior regarding which component receives the input events. While the MouseArea is disabled during drag operations, this layering pattern can be fragile. Consider using a single MouseArea with drag-and-drop handling or restructuring to avoid overlapping interactive areas.
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| var url = drop.urls[0].toString().toLowerCase() | ||
| if (url.endsWith(".iso") || url.endsWith(".raw") || url.endsWith(".xz")) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The drop handling logic only accepts the first URL when multiple files are dropped. If a user drags and drops multiple files, only the first one will be processed without any indication that the others were ignored. Consider either accepting all dropped files or providing feedback that only single file selection is supported.
| id: dropRect | ||
| Layout.fillWidth: true | ||
| Layout.preferredHeight: units.gridUnit * 5 | ||
| border.color: "grey" |
There was a problem hiding this comment.
The border color uses a hardcoded string "grey" which may not adapt to the application's theme or dark mode. Consider using a color from the theme palette or a variable that responds to theme changes for better visual consistency.
| border.color: "grey" | |
| border.color: drivePage.palette.mid |
| DropArea { | ||
| id: dropArea | ||
| anchors.fill: parent | ||
|
|
||
| onDropped: function(drop) { | ||
| if (drop.hasUrls && drop.urls.length > 0) { | ||
| var url = drop.urls[0].toString().toLowerCase() | ||
| if (url.endsWith(".iso") || url.endsWith(".raw") || url.endsWith(".xz")) { | ||
| releases.selectLocalFile(drop.urls[0]) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| FileDialog { | ||
| id: fileDialog | ||
| nameFilters: [ qsTr("Image files") + " (*.iso *.raw *.xz)", qsTr("All files (*)")] | ||
| onAccepted: { | ||
| releases.selectLocalFile(currentFile) | ||
| } | ||
| } |
There was a problem hiding this comment.
The DropArea lacks visual feedback when a drag operation enters or hovers over it. While the label text changes based on containsDrag, there's no border color change, background highlighting, or other visual cue to indicate that the drop zone is active. Consider adding onEntered and onExited handlers to provide visual feedback, such as changing the border color or adding a subtle background tint when a drag is in progress.
Added drag and drop functionality when selecting iso
Screen.Recording.2026-01-08.at.21.21.09.mov
Closes #889