| apol | |
| ngraham |
| Plasma | |
| Discover Software Store |
Kirigami's ListSectionHeader has a default property that points to an embedded RowLayout's
data property, in an attempt (I guess?) to make it simpler to add things to it. However,
this does not work properly, at least not in combination with ActionToolBar. So instead,
override the section header's contentItem so we can specify the right layout properties.
See also https://bugs.kde.org/show_bug.cgi?id=415666
BUG: 415666
The sources page now displays actions properly, but still places them in the overflow menu
when the window gets smaller.
Before:
After:
| No Linters Available |
| No Unit Test Coverage |
| Buildable 20372 | |
| Build 20390: arc lint + arc unit |
Hmm, it doesn't seem to fix the issue for me:
FWIW I've noticed the same phantom overflow menu on the Application page's toolbar:
I've created https://phabricator.kde.org/D26279 for the overflow menu.
I understood this bug to be about certain buttons now being hidden on the sources page, which as far as I could tell was happening due to sizing, which is what this patch fixes.
Ah my mistake. I was confused by the empty overflow issue, which your other patch fixes. I can confirm that this fixes the actual reported issue. :)
However... I wonder if it's also worth fixing ListSectionHeader. It is indeed designed for you to feed it items rather than building your own custom layout, as this was perceived as being more simple. However I can see how this doesn't work at all when you put an ActionToolbar in it since it winds up inside the Rowlayout. I wonder if ultimately the idea was bad and we should just require that you give it your own custom layout... or if maybe it can detect when the item you give it is or has a layout and then replace the RowLayout with it? Or maybe that's a terrible idea.
This does NOT fix BUG: 415666. The distro native sources button is still missing.
Git master with this change:
Correctly showing sources button before commit https://cgit.kde.org/discover.git/commit/?id=89088f7639a63c0c099ccb9c95393f12d83c17b5
The bug mentioned got fixed by this (old Qt doesn't iterate over QList).
I don't see how this would make it look any different. The kirigami visible patch is indeed necessary but for an entirely different reason.
Wish I could verify, but the latest commit causes Discover to crash when visiting the Settings page for me: https://bugs.kde.org/show_bug.cgi?id=415727
The bug mentioned got fixed by this (old Qt doesn't iterate over QList).
I don't see how this would make it look any different. The kirigami visible patch is indeed necessary but for an entirely different reason.
You're right, it does seem to work correctly now. Weird, when I made this I was sure the problem was with the sizing of the ActionToolBar. But maybe I was put off by the overflow button visibility.