Bug 451240 - Screen/Window selection dialog reacts poorly to double click
Summary: Screen/Window selection dialog reacts poorly to double click
Status: RESOLVED FIXED
Alias: None
Product: xdg-desktop-portal-kde
Classification: Plasma
Component: general (show other bugs)
Version: git-master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-07 13:50 UTC by Nicolas Fella
Modified: 2022-07-20 21:39 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.26


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Fella 2022-03-07 13:50:01 UTC
STEPS TO REPRODUCE
1. Start a screen/window share from e.g. OBS 
2. No list item is selected, the share button is disabled
3. Double-click the first list item. Now the Share button is enabled, but no list item is selected
4. Double-click again, now the list item is selected but the share button is disabled

OBSERVED RESULT
The list item selection and the share button are out of sync

EXPECTED RESULT
The list item selection and the share button are out in sync

SOFTWARE/OS VERSIONS
KDE Plasma Version: master
KDE Frameworks Version: master
Qt Version: 5.15-kde

ADDITIONAL INFORMATION
Comment 1 Nate Graham 2022-03-09 15:39:06 UTC
For me, when the list has a single item, it is selected by default, so the share button is enabled. Do you have multiple screens? In that case, we don't pre-select anything.

Regardless, I can reproduce the double-click issue.
Comment 2 Aleix Pol 2022-03-10 01:25:23 UTC
An easy way to test multiscreen if you don't have one is using something like:
$ krfb-virtualmonitor --name "a" --password "banana123" --resolution 1000x1000 --port 2299

It will create a virtual monitor as if you just connected one.
Comment 3 Harald Sitter 2022-07-19 08:17:18 UTC
This is really odd. The model seems to support multi-select but the view does not. Indeed I believe our problems would go away if multiselect wasn't a thing - remove m_selectedRows and instead make it a single toggle that tracks the view's currentindex? @Aleix what do you reckon?
Comment 4 Aleix Pol 2022-07-19 17:49:21 UTC
So I've investigated this and found out the issue. The problem is that we are using onClicked because onToggled isn't available. As we can see on AbstractListItem.qml:

//NOTE: This must stay at 2.0 until KF6 due to retrocompatibility of the "icon" property
import QtQuick.Templates 2.0 as T2

And with onClicked we have less control over how to react to double clicks which means we just do it poorly. I will try and see what would be a good compromise for the moment.
Comment 5 Bug Janitor Service 2022-07-19 17:59:25 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/xdg-desktop-portal-kde/-/merge_requests/123
Comment 6 Bug Janitor Service 2022-07-19 17:59:34 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kirigami/-/merge_requests/608
Comment 7 Aleix Pol 2022-07-20 09:59:52 UTC
Git commit 1b19825b534c523cd7958e35e6338f0661309c13 by Aleix Pol.
Committed on 19/07/2022 at 17:54.
Pushed by apol into branch 'master'.

Try to depend on the newer QQC2 version possible

The first Qt Quick Controls 2 that supports AbstractButton.icon is 2.3,
so just depend on 2.2.
This allows us to have onToggled available which allows us to better
handle double click cases on togglable delegates.

M  +2    -2    src/controls/templates/AbstractListItem.qml

https://invent.kde.org/frameworks/kirigami/commit/1b19825b534c523cd7958e35e6338f0661309c13
Comment 8 Aleix Pol 2022-07-20 10:00:20 UTC
Git commit 35cca308efb3ab9649dffa9fd9c969d535f9aa3e by Aleix Pol.
Committed on 19/07/2022 at 17:58.
Pushed by apol into branch 'master'.

ScreenChooser: Better support handling double-clicked delegates

By using AbstractButton.onToggled instead of .onClicked.

M  +4    -2    src/ScreenChooserDialog.qml

https://invent.kde.org/plasma/xdg-desktop-portal-kde/commit/35cca308efb3ab9649dffa9fd9c969d535f9aa3e