Bug 489745 - Top Objects like channels(snap), permissions(packagekit), flatpak notifications are not visible
Summary: Top Objects like channels(snap), permissions(packagekit), flatpak notificatio...
Status: RESOLVED FIXED
Alias: None
Product: Discover
Classification: Applications
Component: discover (show other bugs)
Version: 6.1.0
Platform: Neon Linux
: HI major
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords: regression
: 489837 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-07-04 16:04 UTC by Soumyadeep Ghosh
Modified: 2024-07-23 12:52 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 6.1.4
Sentry Crash Report:


Attachments
VLC in 6.1.0 plasma discover (254.02 KB, image/png)
2024-07-04 16:04 UTC, Soumyadeep Ghosh
Details
VLC in plasma discover without the bindVisibility (254.40 KB, image/png)
2024-07-04 16:29 UTC, Soumyadeep Ghosh
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Soumyadeep Ghosh 2024-07-04 16:04:51 UTC
Created attachment 171375 [details]
VLC in 6.1.0 plasma discover

SUMMARY
Top Objects, like the channels button, or the permission notice for packagekit, or flatpak eol notifications aren't visible anymore

STEPS TO REPRODUCE
1. Open Discover
2. Click on any app (deb, snap, flatpak)
3. for deb no full permission notice, for snaps no channel button, for flatpak no eol notice or such objects

OBSERVED RESULT

No buttons there

EXPECTED RESULT

Expected these buttons to show up properly

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: KDE Neon
(available in About System)
KDE Plasma Version: 6.1.1
KDE Frameworks Version: 6.3.0
Qt Version: 6.7.0

ADDITIONAL INFORMATION

The main problem seems to be in the visibility binding from here. 
https://invent.kde.org/plasma/discover/-/blob/master/discover/qml/ApplicationPage.qml?ref_type=heads#L426-437


function bindVisibility() {
                hasVisibleObjects = Qt.binding(() => {
                    for (let i = 0; i < topObjectsRepeater.count; i++) {
                        const loader = topObjectsRepeater.itemAt(i);
                        const item = loader.item;
                        if (item?.visible) {
                            return true;
                        }
                    }
                    return false;
                });
            }

Here, no matter what the visibility is set for the item,
item.visible always returns false. To temporarily, we should remove this as it's done with bottomObjects.
Comment 1 Soumyadeep Ghosh 2024-07-04 16:29:27 UTC
Created attachment 171376 [details]
VLC in plasma discover without the bindVisibility
Comment 2 Bug Janitor Service 2024-07-07 23:09:23 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/875
Comment 3 ratijas 2024-07-08 13:41:17 UTC
Hi Ghosh,

your screenshots are featuring VLC from two different sources, one of which ("ubuntu-jammy-universe") could not possibly have topObjects). Are you sure there is a bug?  The binding for hasVisibleObjects property only affects the top margin, not the transient visibility of the container.
Comment 4 ratijas 2024-07-08 13:52:59 UTC
Ah, I see the erroneous `visible: hasVisibleObjects` line has been added in https://invent.kde.org/plasma/discover/-/commit/bca6b4beefcda1fcb80b225c8cd43868cf8321c4#note_985864
Comment 5 Nate Graham 2024-07-10 18:41:44 UTC
Oops, so sorry for the regression!
Comment 6 Aleix Pol 2024-07-19 00:57:47 UTC
Git commit 101c01494ede10659740827c63dac874bf18f121 by Aleix Pol Gonzalez, on behalf of Aleix Pol.
Committed on 19/07/2024 at 00:54.
Pushed by apol into branch 'master'.

ApplicationPage: Do not use Item.visible to calculate the visibility of the parent

Having the parent visible=false will make all children visible:false so
it becomes a silly exercise.

Instead, use a property made for this that doesn't have other
connotations.

It kind of feels like we are workarounding a bug in the layout class but
this is needs addressing now.

M  +48   -1    discover/DiscoverDeclarativePlugin.cpp
M  +17   -10   discover/qml/ApplicationPage.qml
M  +1    -0    libdiscover/backends/DummyBackend/CMakeLists.txt
M  +6    -0    libdiscover/backends/DummyBackend/DummyResource.cpp
M  +1    -0    libdiscover/backends/DummyBackend/DummyResource.h
C  +3    -4    libdiscover/backends/DummyBackend/qml/DummyTop.qml [from: libdiscover/backends/FlatpakBackend/qml/FlatpakAttention.qml - 063% similarity]
A  +6    -0    libdiscover/backends/DummyBackend/resources.qrc
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakAttention.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakEolReason.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakOldBeta.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakRemoveData.qml
M  +1    -2    libdiscover/backends/FlatpakBackend/qml/PermissionsList.qml
M  +1    -2    libdiscover/backends/PackageKitBackend/qml/DependenciesButton.qml
M  +1    -0    libdiscover/backends/PackageKitBackend/qml/PackageKitPermissions.qml
M  +1    -1    libdiscover/backends/SnapBackend/qml/ChannelsButton.qml
M  +1    -1    libdiscover/backends/SnapBackend/qml/PermissionsButton.qml

https://invent.kde.org/plasma/discover/-/commit/101c01494ede10659740827c63dac874bf18f121
Comment 7 Aleix Pol 2024-07-19 01:05:43 UTC
Git commit 15a277e189cdeedb8feb32428c1375d386f9a9dc by Aleix Pol Gonzalez.
Committed on 19/07/2024 at 01:02.
Pushed by apol into branch 'Plasma/6.1'.

ApplicationPage: Do not use Item.visible to calculate the visibility of the parent

Having the parent visible=false will make all children visible:false so
it becomes a silly exercise.

Instead, use a property made for this that doesn't have other
connotations.

It kind of feels like we are workarounding a bug in the layout class but
this is needs addressing now.


(cherry picked from commit 101c01494ede10659740827c63dac874bf18f121)

Co-authored-by: Aleix Pol <aleixpol@kde.org>

M  +48   -1    discover/DiscoverDeclarativePlugin.cpp
M  +17   -10   discover/qml/ApplicationPage.qml
M  +1    -0    libdiscover/backends/DummyBackend/CMakeLists.txt
M  +6    -0    libdiscover/backends/DummyBackend/DummyResource.cpp
M  +1    -0    libdiscover/backends/DummyBackend/DummyResource.h
C  +3    -4    libdiscover/backends/DummyBackend/qml/DummyTop.qml [from: libdiscover/backends/FlatpakBackend/qml/FlatpakAttention.qml - 063% similarity]
A  +6    -0    libdiscover/backends/DummyBackend/resources.qrc
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakAttention.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakEolReason.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakOldBeta.qml
M  +1    -1    libdiscover/backends/FlatpakBackend/qml/FlatpakRemoveData.qml
M  +1    -2    libdiscover/backends/FlatpakBackend/qml/PermissionsList.qml
M  +1    -2    libdiscover/backends/PackageKitBackend/qml/DependenciesButton.qml
M  +1    -0    libdiscover/backends/PackageKitBackend/qml/PackageKitPermissions.qml
M  +1    -1    libdiscover/backends/SnapBackend/qml/ChannelsButton.qml
M  +1    -1    libdiscover/backends/SnapBackend/qml/PermissionsButton.qml

https://invent.kde.org/plasma/discover/-/commit/15a277e189cdeedb8feb32428c1375d386f9a9dc
Comment 8 Harald Sitter 2024-07-23 12:52:44 UTC
*** Bug 489837 has been marked as a duplicate of this bug. ***