[Qt] Add DetailedList widget

[corranwebster]

Qt doesn't implement pull-to-refresh, or slide to reveal actions, so all actions are implemented via a context menu, including Refresh. Getting the multi-line layout required using a QStyledItemDelegate for the drawing, and for speed simply drawing using QPainter - it should follow the style of the app with no further tweaking.

Thanks to @johnzhou721 for lots of help with debugging the tests.

Ref #3914.

To Do:

• screenshot.
• coverage.
• resolution of Add Pre-Change Notifications to Sources #4028 one way or another
• resolution of ListSource.__setitem__ notifies "insert" rather than "change" #4029 one way or another

In terms of #4028 I'm going to assume that we're not going to be adding that soon, since it is a fairly substantial change to how end users are asked to do notifications on data sources. If the current approach causes problems, then we can go back and address #4028 more urgently.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[johnzhou721]

@corranwebster My 2 cents is that it might be beneficial to have buttons below the DetailedList with things like "Refresh", "[Primary Action Text]", and "[Secondary Action Text]" because KDE discourages things that can only be accessed in context menus -- we could enable/disable the buttons as we select/deselect.

FYI -- the page I'm linking to is linked from this page which is KDE's visual design group's guidelines for those that want to join; in it it referneces my first link as something to read first which is KDE's design group's concencus on common issues, including no context-menu-only actions.

Still if this is a bit too much work, it can be Russ' call to land a context-menu-only version, with "adding buttons below" issue kept track of as a first pass (since Qt backend is intended to be pre-alpha at this stage, anyways).

[corranwebster]

@corranwebster My 2 cents is that it might be beneficial to have buttons below the DetailedList with things like "Refresh", "[Primary Action Text]", and "[Secondary Action Text]" because KDE discourages things that can only be accessed in context menus -- we could enable/disable the buttons as we select/deselect.

FYI -- the page I'm linking to is linked from this page which is KDE's visual design group's guidelines for those that want to join; in it it referneces my first link as something to read first which is KDE's design group's concencus on common issues, including no context-menu-only actions.

That's fair - I was following the macOS version as closely as I could when designing it (allowing for the fact that Qt doesn't seem to support pull-down to refresh). What would help is if you know of an example of an app with a widget that behaves like the detailed list is supposed to so I can see what is considered standard on KDE.

Obvious choices would be things like a "..." or hamburger button to indicate a place to click to get the menu or buttons inside the widget that appear when you activate it or hover over it. I think I'd like to avoid buttons outside the list itself unless that's the preferred style, in which case it would be good to see what it looks like. For an example of what I mean, macOS used to have a lot of tables like this:

with a fairly standard-ish set of buttons at the bottom.

The other approach to take is that this is just a widget and not the full application, and leave it to the app developer to bring things in-line with best practices however works best for their application: whether it's actual menubar menu items which track the selection, or buttons which provide the same actions which appear in a side-panel with more details about the selected item... or some extra buttons below the widget.

Still if this is a bit too much work, it can be Russ' call to land a context-menu-only version, with "adding buttons below" issue kept track of as a first pass (since Qt backend is intended to be pre-alpha at this stage, anyways).

In terms of difficulty, this widget has already got an ItemDelegate with custom rendering of text and images, so it's not a huge lift to add additional rendering inside the cell to draw buttons... although avoiding too much layout work would be good. It basically means adding event handling to the current ItemDelegate.

Testing may be a pain, though.

I think I'd be interested to hear what Russ thinks.

[johnzhou721]

2 examples --

Discover / built-in app store:

Alligator / RSS feed reader:

It seems that actions are put on the right side for built-in app store, and the "refresh" button is in a top toolbar.

Those toolbars are attached to the main window, so yes, I think "it's just a widget" interpretation will make sense. We might finess this by automatically adding the Refresh action to the right side of the window toolbar when we're in a MainWindow, but to avoid scopecreep I think we should just get primary and secondary drawn for now.

But definitely drawing in primary and secondary things on the widget items itself will be beneficial. It's usually a small icon... but I don't suppose text will be too obtrusive there.

[corranwebster]

I think this is ready for review.

• There is a test failing and I'm not sure why; help would be appreciated. I am not sure why the text widget can't get focus on the Xwindows version but can on the Wayland version. It seems orthogonal to the behaviour of the new widget - it is failing with neither widget having focus.
• Punting on Add Pre-Change Notifications to Sources #4028 for now.

[freakboy3742]

I think I'd be interested to hear what Russ thinks.

I think the key detail here is Qt's lack of a "pull to refresh". No other platform currently needs to add an explicit "refresh" button; that means that "just put a DetailedList widget in my layout" needs to expose some sort of refresh capability.

Right now, I'm happy with that to be a context menu (on the basis of the perfect being the enemy of the good); however, if the Qt style guide says context menus should be avoided (and I can certainly sympathise with that as design guidance), then a button bar included as part of the widget makes some sense.

The only thing that stood out in my testing is that the popup menu appeared in an odd location - no matter where I clicked, it popped up in the top left corner of the window content. I presume that isn't the expected behaviour; I'm not sure if that's a bug that has slipped in, or if it's a quirk of my own VM setup (I'm running Fedora 42 in a Parallels session on macOS ARM64, using a trackpad, so right clicks are already the subject of some oddities).

As for the X11 failure - I can't explain that either. I agree it seems entirely orthogonal to the change here; my only thought is that it's some sort of issue where the new DetailedList test is leaving the app in an odd state (e.g., with the app window not having explicit focus), resulting in the text input test failing down the line... but that's just a theory.

[johnzhou721]

@freakboy3742 The thing you are observing is that on Wayland, popups at mouse position does not neccessarily work properly

EDIT: Button bar included with the widget doesn't make as much sense as putting it in the toolbar, however those apps putting it in the toolbar are convergent apps that works on both desktop and mobile where real estate is presumably more important. I'm personally also comfotable with a small bar on top displaying the refresh rather than in the toolbar.

@corranwebster You may find https://community.kde.org/Guidelines_and_HOWTOs/Wayland_Porting_Notes#Context_Menu helpful on this.

Since it's X11 only... perhaps the popup not dismissing correctly?

[corranwebster]

@johnzhou721 @freakboy3742 Looking at the failure the tests before it were fairly general property tests (test_enable_noop and test_flex_widget_size) not something complex involving menus or running asynchronously. I don't have easy access to an X-windows based KDE system, so I've put a little probe into the CI to see what does have the focus in that test, but there's no reason that it should fail that compared to any other widget.

[johnzhou721]

It’s a detailed list test but failing with a text input probe, so not sure why. I will investigate this when I get home today.

[corranwebster]

I've put a little probe into the CI to see what does have the focus in that test

The answer: nothing.

----------------------------- Captured stdout call -----------------------------
None
=========================== short test summary info ============================

ETA: Thought – I'm old enough to have used X-Windows Solaris boxes back in the day with "focus follows mouse pointer" behaviour but I assume that any modern KDE-based system isn't doing that.

[johnzhou721]

@corranwebster

So it's the fact that when there's a QListView on the page, TextInput seems to be not focussing properly.

This is in CI and I can't replicate this issue locally on X11 KDE; so I can only make suggestions.

Can you try adding delay=1 as a keyword argument to the previous line of redraw before we're failing? It may or may not work, but if it does, we need an adaptive approach to wait for focus.

[corranwebster]

I've tried adding a delay to allow the focus to settle. I've also set the focus policy so that the QListView should never accept keyboard focus (which may not be what we want long-term). Still no luck.

[johnzhou721]

@corranwebster OK, so then let's get the last 2 commits reverted.

Next possible hypothesis: The test window is not focussed. Can you peek into test_focus and add a print statement printing toga.App.app.current_window then show the results?

[corranwebster]

----------------------------- Captured stdout call -----------------------------
Current Toga window: None
=========================== short test summary info ============================

So the problem seems to be that there isn't an active window in the App.

If I have time today I'll look into what is needed to make sure that the window with the test widget is active.

[johnzhou721]

@corranwebster The issue is line 230 of toga_qt/widgets/detailedlist.py. Showing the widget then is not good, because it pops up as a separate window and then gets parented, which steals the focus away on X11 but not Wayland, which is asynchronous. Base widget, container etc. has logic for managing visibility in widgets.

[corranwebster]

@freakboy3742 I think this is finally ready for a review.

[freakboy3742]

Nice work tracking down the test window focus issue!

There's clearly a longer term issue around working out how to expose better interfaces for refresh; but otherwise this looks great. Thanks for another great widget!

[johnzhou721]

Refs #4097 for the context menu issue.