From ec6407fb1e602d13027abcd19280a0abb0f1fb6c Mon Sep 17 00:00:00 2001 From: bbedward Date: Thu, 24 Sep 2026 21:03:26 -0400 Subject: [PATCH] cc: drop the detail dialog scroller so pages scroll to their end fixes #3562 --- .../ControlCenter/Components/CcDetailPage.qml | 142 ++++++-------- quickshell/tests/qml/media-artwork.qml | 184 ------------------ 2 files changed, 61 insertions(+), 265 deletions(-) delete mode 100644 quickshell/tests/qml/media-artwork.qml diff --git a/quickshell/Modules/ControlCenter/Components/CcDetailPage.qml b/quickshell/Modules/ControlCenter/Components/CcDetailPage.qml index ee26bbc74..253052369 100644 --- a/quickshell/Modules/ControlCenter/Components/CcDetailPage.qml +++ b/quickshell/Modules/ControlCenter/Components/CcDetailPage.qml @@ -32,9 +32,13 @@ FocusScope { readonly property bool animationsEnabled: CcMetrics.animationsEnabled && !SettingsData.reduceMotion readonly property bool transitioning: enterPending || enterAnimation.running readonly property var pageItem: pageLoader.item - readonly property real contentHeight: Math.max(minimumContentHeight, CcMetrics.preferredDetailHeight(shownSection, pageItem?.preferredHeight ?? 0)) - readonly property real minimumHeight: preferredHeight - contentHeight + Math.min(contentHeight, CcMetrics.detailMinContentHeight) - readonly property real preferredHeight: panel.height + CcMetrics.detailDialogPadding * 2 + readonly property real pageHeight: CcMetrics.preferredDetailHeight(shownSection, pageItem?.preferredHeight ?? 0) + readonly property real contentHeight: Math.max(minimumContentHeight, pageHeight) + readonly property real chromeHeight: header.height + footer.height + CcMetrics.detailDialogPadding * 2 + readonly property real maximumHeight: height - topInset - CcMetrics.detailDialogInset + // plugin detail content may not scroll itself, so the panel grows to fit it + readonly property bool pageScrollsItself: !shownSection.startsWith("plugin_") + readonly property real minimumHeight: chromeHeight + (pageScrollsItself ? Math.min(contentHeight, CcMetrics.detailMinContentHeight) : pageHeight) readonly property string title: { const own = pageItem?.title ?? ""; if (own) @@ -138,7 +142,6 @@ FocusScope { root.opacity = enterPending ? 0 : 1; dialogSurface.entryScale = enterPending ? CcMetrics.popupEnterScale : 1; shownSection = section; - dialogViewport.contentY = 0; pageLoader.sourceComponent = _componentFor(section); (pageItem ?? root).forceActiveFocus(); } @@ -212,7 +215,7 @@ FocusScope { x: CcMetrics.detailDialogInset y: root.topInset width: Math.max(0, root.width - CcMetrics.detailDialogInset * 2) - height: Math.max(0, Math.min(root.preferredHeight, root.height - root.topInset - CcMetrics.detailDialogInset)) + height: Math.max(0, Math.min(root.chromeHeight + root.contentHeight, root.maximumHeight)) radius: Theme.cornerRadiusXL color: Theme.readableSurface border.width: Theme.layerOutlineWidth @@ -225,98 +228,75 @@ FocusScope { acceptedButtons: Qt.AllButtons } - DankFlickable { - id: dialogViewport + Item { + id: panel anchors.fill: parent anchors.margins: CcMetrics.detailDialogPadding - contentWidth: width - contentHeight: panel.height - clip: true - interactive: contentHeight > height - wheelEnabled: interactive - readonly property Item windowFocusItem: root.Window.window?.activeFocusItem ?? null - onWindowFocusItemChanged: { - const item = windowFocusItem; - if (!root.section || !item?.activeFocusOnTab || !root.containsItem(item) || !interactive) - return; - const top = item.mapToItem(contentItem, 0, 0).y; - const bottom = top + item.height; - const target = top < contentY ? top : Math.max(contentY, bottom - height); - contentY = Math.max(0, Math.min(contentHeight - height, target)); + Item { + id: header + anchors.left: parent.left + anchors.right: parent.right + anchors.top: parent.top + height: CcMetrics.pageHeaderHeight + + StyledText { + anchors.left: parent.left + anchors.leftMargin: Theme.spacingS + anchors.right: headerSlot.left + anchors.rightMargin: Theme.spacingM + anchors.verticalCenter: parent.verticalCenter + text: root.title + font.pixelSize: CcMetrics.pageTitleSize + color: Theme.surfaceText + elide: Text.ElideRight + horizontalAlignment: Text.AlignLeft + } + + Item { + id: headerSlot + anchors.right: parent.right + anchors.verticalCenter: parent.verticalCenter + width: childrenRect.width + height: parent.height + } + } + + Loader { + id: pageLoader + anchors.left: parent.left + anchors.right: parent.right + anchors.top: header.bottom + anchors.bottom: footer.top + active: root.shownSection !== "" + onLoaded: { + const actions = item.headerActions ?? null; + if (actions) + actions.parent = headerSlot; + } } Item { - id: panel + id: footer + anchors.left: parent.left + anchors.right: parent.right + anchors.bottom: parent.bottom + height: closeButton.height + Theme.spacingS - width: dialogViewport.width - height: header.height + root.contentHeight + footer.height - - Item { - id: header - anchors.left: parent.left - anchors.right: parent.right - anchors.top: parent.top - height: CcMetrics.pageHeaderHeight - - StyledText { - anchors.left: parent.left - anchors.leftMargin: Theme.spacingS - anchors.right: headerSlot.left - anchors.rightMargin: Theme.spacingM - anchors.verticalCenter: parent.verticalCenter - text: root.title - font.pixelSize: CcMetrics.pageTitleSize - color: Theme.surfaceText - elide: Text.ElideRight - horizontalAlignment: Text.AlignLeft - } - - Item { - id: headerSlot - anchors.right: parent.right - anchors.verticalCenter: parent.verticalCenter - width: childrenRect.width - height: parent.height - } - } - - Loader { - id: pageLoader - anchors.left: parent.left - anchors.right: parent.right - anchors.top: header.bottom - anchors.bottom: footer.top - active: root.shownSection !== "" - onLoaded: { - const actions = item.headerActions ?? null; - if (actions) - actions.parent = headerSlot; - } - } - - Item { - id: footer - anchors.left: parent.left + DankButton { + id: closeButton anchors.right: parent.right anchors.bottom: parent.bottom - height: closeButton.height + Theme.spacingS - - DankButton { - id: closeButton - anchors.right: parent.right - anchors.bottom: parent.bottom - text: I18n.tr("Close") - onClicked: root.backRequested() - } + text: I18n.tr("Close") + onClicked: root.backRequested() } } } Item { id: menuOverlay - anchors.fill: dialogViewport + anchors.fill: panel } } diff --git a/quickshell/tests/qml/media-artwork.qml b/quickshell/tests/qml/media-artwork.qml deleted file mode 100644 index 936c44d99..000000000 --- a/quickshell/tests/qml/media-artwork.qml +++ /dev/null @@ -1,184 +0,0 @@ -import QtQuick -import QtTest -import Quickshell -import qs.Common -import qs.Services -import qs.Modules.OSD -import qs.Modules.DankDash -import qs.DankCommon.Common as DC - -ShellRoot { - id: root - property var cover: null - property int coverChanges: 0 - property bool checkedNewMetadata: false - property bool previousArtWasReady: false - Component.onCompleted: { - Quickshell.watchFiles = false; - DC.Style.theme = Theme; - DC.Style.settings = SettingsData; - DC.I18n.backend = I18n; - } - TestCase { - id: input - when: false - name: "media-artwork" - } - FloatingWindow { - visible: true - implicitWidth: 800 - implicitHeight: 700 - MediaPlayerTab { - id: tab - anchors.fill: parent - } - } - MediaPlaybackOSD { - id: osd - modelData: Quickshell.screens[0] - autoHideInterval: 10000 - } - Connections { - target: root.cover - function onArtUrlChanged() { - root.coverChanges++; - } - } - Connections { - target: MprisController.activePlayer - function onTrackTitleChanged() { - root.checkedNewMetadata = true; - root.previousArtWasReady = TrackArtService.artReadyFor(MprisController.activePlayer); - } - } - function find(item, predicate) { - if (!item) - return null; - if (predicate(item)) - return item; - for (const child of item.children ?? []) { - const found = find(child, predicate); - if (found) - return found; - } - return null; - } - function check(value, message) { - if (!value) - throw new Error(message); - } - function waitFor(condition, message) { - const deadline = Date.now() + 20000; - while (!condition()) { - check(Date.now() < deadline, "timed out: " + message); - input.wait(10); - } - } - function committed(track) { - const player = MprisController.activePlayer; - return player.trackTitle === "Track " + track && TrackArtService.artReadyFor(player) && cover.artUrl === TrackArtService.resolvedArtUrl; - } - // a rapid skip commits the previous cover for a moment, so the state has to hold before it counts as settled - function settle(track) { - for (let attempt = 0; attempt < 40; attempt++) { - input.wait(100); - if (!committed(track)) - continue; - input.wait(200); - if (committed(track)) - return; - } - } - function checkTrack(track, channel) { - settle(track); - const player = MprisController.activePlayer; - check(player.trackTitle === "Track " + track, "expected player track"); - check(TrackArtService.artReadyFor(player), "current track artwork ready on track " + track); - check(root.cover.artUrl === TrackArtService.resolvedArtUrl, "dash uses committed artwork"); - check(MediaAccentService.artUrl === root.cover.artUrl, "accent uses displayed artwork"); - waitFor(() => { - const channels = [MediaAccentService.accent.r, MediaAccentService.accent.g, MediaAccentService.accent.b]; - return channels[channel] > 0.6 && channels.filter((value, index) => index !== channel).every(value => value < 0.3); - }, "accent matches cover pixels on track " + track); - check(osd.shouldBeVisible, "transport keeps OSD open on track " + track); - const image = root.find(root.cover, item => item.status !== undefined && item.sourceSize !== undefined); - check(image.visible && image.retainWhileLoading, "cover stays painted during loading"); - } - Timer { - interval: 0 - running: true - onTriggered: { - try { - root.cover = root.find(tab, item => item.artPixelSize !== undefined); - root.check(!!root.cover, "dash artwork component"); - root.waitFor(() => !!MprisController.activePlayer && TrackArtService.artReadyFor(MprisController.activePlayer), "player discovered with artwork"); - const player = MprisController.activePlayer; - SessionData.suppressOSD = false; - osd.show(); - input.wait(300); - root.checkTrack(1, 0); - root.check(root.coverChanges <= 1, "initial cover is published at most once"); - root.coverChanges = 0; - root.checkedNewMetadata = false; - player.next(); - root.checkTrack(2, 2); - root.check(root.checkedNewMetadata && !root.previousArtWasReady, "previous artwork is not ready for new metadata"); - root.check(root.coverChanges === 1, "duplicate cover files do not reload dash"); - root.coverChanges = 0; - const previous = root.find(osd.contentLoader.item, item => item.objectName === "previousTrack"); - // the previous button rewinds instead of skipping back once playback passes eight seconds - player.position = 0; - input.wait(50); - previous.click(); - root.checkTrack(1, 0); - root.check(root.coverChanges === 1, "back publishes one matching cover"); - root.coverChanges = 0; - player.next(); - input.wait(20); - player.next(); - root.checkTrack(3, 1); - root.check(root.coverChanges === 1, "rapid skips publish only the latest cover"); - root.coverChanges = 0; - player.next(); - input.wait(130); - player.next(); - root.checkTrack(5, 2); - const smallerArt = TrackArtService.resolvedArtUrl; - root.check(root.coverChanges === 1, "late file completion cannot replace newer art"); - player.next(); - root.checkTrack(6, 1); - // Chrome attaches the previous track's late, smaller cover to the new track, and the two probes finish in either order - const largerArt = TrackArtService.resolvedArtUrl; - const key = TrackArtService._pendingArtKey; - const sha = url => url.slice(url.lastIndexOf("art_") + 4); - const forgetArt = () => { - TrackArtService.artwork = { - url: "", - key: "", - source: "", - colors: [] - }; - TrackArtService._largestArt = { - key: "", - url: "", - side: 0 - }; - }; - forgetArt(); - TrackArtService._artProbed(player.trackArtUrl, key, TrackArtService._requestSerial, sha(largerArt) + " 128", 0); - root.waitFor(() => TrackArtService.resolvedArtUrl === largerArt, "first cover commits"); - TrackArtService._requestSerial++; - TrackArtService._artProbed(player.trackArtUrl, key, TrackArtService._requestSerial, sha(smallerArt) + " 100", 0); - root.check(TrackArtService.resolvedArtUrl === largerArt && !TrackArtService._candidateArt, "late smaller cover does not replace the current one"); - forgetArt(); - TrackArtService._artProbed(player.trackArtUrl, key, TrackArtService._requestSerial, sha(smallerArt) + " 100", 0); - TrackArtService._artProbed("file:///superseded", key, TrackArtService._requestSerial - 1, sha(largerArt) + " 128", 0); - root.waitFor(() => TrackArtService.resolvedArtUrl === largerArt, "larger cover wins when its probe finishes after the late cover's"); - console.log("FIXTURE_PASS artwork readiness, matching accent, no duplicate cover reload, back, rapid skips, late completion, stale previous covers and constant backdrop opacity"); - } catch (error) { - console.error("FIXTURE_FAIL", error.message); - } - Qt.quit(); - } - } -}