Bug 395455

Summary: Discover buttons install/remove/launch disappear
Product: [Applications] Discover Reporter: Michał Dybczak <michal.dybczak>
Component: discoverAssignee: Aleix Pol <aleixpol>
Status: RESOLVED FIXED    
Severity: normal CC: aalexandera, djvbmd, nate
Priority: VHI    
Version: 5.13.0   
Target Milestone: ---   
Platform: Manjaro   
OS: Linux   
Latest Commit: Version Fixed In: 5.14.0
Attachments: Missing buttons

Description Michał Dybczak 2018-06-15 21:04:37 UTC
Created attachment 113358 [details]
Missing buttons

Steps to reproduce:
1. Open Discover
2. Go to the installed top program (in my case Krita), see Launch and Remove buttons, they are displayed correctly at this stage.
3. Go to the some not installed program (in my case Digicam), see Install button which is displayed correctly at this stage.
4. Go back to main programs menu and go again to the first or others programs windows, they don't have the buttons to install/launch/remove program

Works every time. In fact, all it takes is to go to the few random top programs and in second or third view buttons are gone.
I tried it on test, clean user and the issue is visible there as well.
Comment 1 Nate Graham 2018-06-17 18:56:14 UTC
Confirmed.

Workaround: resize the window even a bit, and the layout corrects itself.
Comment 2 Michał Dybczak 2018-06-17 20:39:46 UTC
Indeed, by resizing window buttons appeared (together with a layout change) giving quite nice 3 column look. Awesome.
Comment 3 Nate Graham 2018-06-22 15:19:25 UTC
Yep, Discover's layout is dynamic. As you make the window wider or narrower, it adjusts to better present the content!
Comment 4 Michał Dybczak 2018-06-23 08:39:55 UTC
There were lately some updates in Discover's kirigami (Plasma 5.13.1), or at least I think I saw something like that. Is it fixed? I don't know how to go back to those bugged window size to test it... maybe aside deleting conf.
Comment 5 Nate Graham 2018-06-23 13:18:15 UTC
You can reproduce the bug by reducing the window size so that it shows a two-pane layout, then repeatedly navigate from one app page back to the list, then back to another. Try searching too.

FWIW we're testing out a potential fix for the issue: https://phabricator.kde.org/D13663
Comment 6 Michał Dybczak 2018-06-23 15:41:24 UTC
Ah thanks. In two panel mode bug still exists.
Comment 7 Marco Martin 2018-06-26 16:33:43 UTC
Git commit 26b8bdea24c3930533be9abdef6a9202b507d1bc by Marco Martin.
Committed on 26/06/2018 at 16:33.
Pushed by mart into branch 'master'.

Refactor the Global ToolBar concept

Summary:
The recomended toolbar to be used for kirigami applications used to need to be explicitly instantiated, and had an internal flickable that tried hard to be synchronized with the main PageRow.
but this is pretty much impossible to have it glitch-free, it will there always be an edge case in which the two flickables gets out of sync, which is a strong sign of bad architecture.

Page toolbars are now moved on top of Page itself, so on the same Flickable as the PageRow itself. (a global header is still used when we are in breadcrumbs mode)
Is now also now possible to have multiple instances of PageRows, each one with its own globalToolBar. So, it's now also possible for AbstractApplicationHeader (and any subclass) to trach a different PageRow than the global one in ApplicationWindow.

also, there have been requests to be possible while keeping the default toolbar, have a custom global one that goes on top of it (ApplicationWindow's header item)

also, i wanted to have a way to make it possible to switch on the fly between a mobile-like and a desktop-like toolbar for further convergence plans

in the future, it will be possible for pages to put their own component to replace the default toolbar

Test Plan:
tested all possible combinations of both the new way and the legacy compatibility mode.

ApplicationHeader and ToolBarApplicationHeader still work if used explicitly.

The patch shouldn't have significant visible UI changes.

there is still an explicit FIXME tough i want to do it in a different turn, as would need a new component

Reviewers: #kirigami, ngraham

Reviewed By: #kirigami, ngraham

Subscribers: apol, IlyaBizyaev, ngraham, davidedmundson, plasma-devel

Tags: #kirigami

Differential Revision: https://phabricator.kde.org/D13663

M  +51   -19   examples/gallerydata/contents/ui/DesktopExampleApp.qml
M  +48   -14   examples/gallerydata/contents/ui/ExampleApp.qml
M  +1    -1    examples/gallerydata/contents/ui/gallery/LayersGallery.qml
M  +2    -2    examples/simpleexamples/simpleChatApp.qml
M  +6    -0    kirigami.qrc
M  +15   -3    src/controls/AbstractApplicationHeader.qml
M  +1    -0    src/controls/ApplicationWindow.qml
M  +9    -1    src/controls/OverlayDrawer.qml
M  +72   -9    src/controls/Page.qml
M  +67   -5    src/controls/PageRow.qml
M  +1    -0    src/controls/ToolBarApplicationHeader.qml
A  +42   -0    src/controls/private/AbstractPageHeader.qml     [License: LGPL (v2+)]
A  +48   -0    src/controls/private/PageRowGlobalToolBarStyleGroup.qml     [License: LGPL (v2+)]
A  +96   -0    src/controls/private/PageRowGlobalToolBarUI.qml     [License: LGPL (v2+)]
M  +7    -6    src/controls/private/PrivateActionToolButton.qml
A  +47   -0    src/controls/private/TitlesPageHeader.qml     [License: LGPL (v2+)]
A  +119  -0    src/controls/private/ToolBarPageHeader.qml     [License: LGPL (v2+)]
M  +40   -33   src/controls/templates/AbstractApplicationHeader.qml
M  +22   -21   src/controls/templates/ApplicationHeader.qml
M  +17   -11   src/controls/templates/OverlayDrawer.qml
M  +2    -3    src/controls/templates/private/BackButton.qml
M  +2    -4    src/controls/templates/private/ForwardButton.qml
M  +3    -1    src/enums.h
M  +3    -0    src/styles/org.kde.desktop/AbstractApplicationHeader.qml
M  +13   -5    src/styles/org.kde.desktop/OverlayDrawer.qml

https://commits.kde.org/kirigami/26b8bdea24c3930533be9abdef6a9202b507d1bc
Comment 8 Aleix Pol 2018-06-26 17:30:55 UTC
Git commit d5f7c102854c41e4ee9eb00cfb979993f2396233 by Aleix Pol.
Committed on 26/06/2018 at 17:29.
Pushed by apol into branch 'master'.

Adapt to new syntax for the header

Fixes weird behavior on the Kirigami header

M  +1    -1    CMakeLists.txt
M  +2    -9    discover/qml/DiscoverWindow.qml

https://commits.kde.org/discover/d5f7c102854c41e4ee9eb00cfb979993f2396233
Comment 9 Nate Graham 2018-07-30 23:09:47 UTC
*** Bug 396067 has been marked as a duplicate of this bug. ***