Differential D13931 Diff 37753 src/kcm/package/contents/ui/Advanced.qml

Changeset View

Standalone View

View Options

src/kcm/package/contents/ui/Advanced.qml

Show All 17 Lines1/*
18 along with this program. If not, see <http://www.gnu.org/licenses/>. 18​ along with this program. If not, see <http://www.gnu.org/licenses/>.
19*/ 19​*/
20 20​
21import QtQuick 2.0 21​import QtQuick 2.0
22import QtQuick.Layouts 1.1 22​import QtQuick.Layouts 1.1
23import QtQuick.Controls 1.3 23​import QtQuick.Controls 1.3
24 24​
25import org.kde.plasma.private.volume 0.1 25​import org.kde.plasma.private.volume 0.1
26​import org.kde.kcoreaddons 1.0 as KCoreAddons
27​
26 28​
27ScrollView { 29​ScrollView {
28 id: scrollView 30​ id: scrollView
29 31​
30 ColumnLayout { 32​ ColumnLayout {
31 Component.onCompleted: { 33​ Component.onCompleted: {
32 // Normal binding causes binding loops 34​ // Normal binding causes binding loops
33 width = Qt.binding(function() { 35​ width = Qt.binding(function() {
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Line(s)
88 90​
89 Label { 91​ Label {
90 Layout.alignment: Qt.AlignHCenter 92​ Layout.alignment: Qt.AlignHCenter
91 enabled: false 93​ enabled: false
92 font.italic: true 94​ font.italic: true
93 text: i18n("Requires 'module-gconf' PulseAudio module") 95​ text: i18n("Requires 'module-gconf' PulseAudio module")
94 visible: moduleManager.settingsSupported && moduleManager.loadedModules.indexOf("module-gconf") == -1 96​ visible: moduleManager.settingsSupported && moduleManager.loadedModules.indexOf("module-gconf") == -1
95 } 97​ }
98​
99​ Header {
100​ Layout.fillWidth: true
101​ text: i18n("Speaker Placement and Testing")
102​ }
103​
104​ RowLayout {
105​ Layout.margins: units.gridUnit / 2
droscaUnsubmitted
Done

newline

drosca: newline
106​ visible: sinks.count > 1
107​
108​ Label {
109​ text: i18nc("@label", "Output:")
110​ font.bold: true
111​ }
112​
113​ ComboBox {
114​ id: sinks
115​
droscaUnsubmitted
Done

So this will show this grid as many times as you have sinks below each other and scrollable in ListView without any indication which grid is for which device?
That doesn't look good to me.

What about showing the grid just once and adding a combobox with device selection under (or above)?

Also, indentation is off here:

delegate: Grid {
    // ...
}
drosca: So this will show this grid as many times as you have sinks below each other and scrollable in…
116​ property var pulseObject: null
117​
118​ Layout.fillWidth: true
119​ textRole: "Description"
120​ model: SinkModel {
droscaUnsubmitted
Not Done

What is this doing?

onDataChanged will be triggered only when some property of data in model changes, and in that case you overwritten the binding that is set in grid, so grid.pulseObject will no longer be updated when ComboBox current index is changed.

I don't think this is needed at all.

drosca: What is this doing? onDataChanged will be triggered only when some property of data in model…
nicolasfellaAuthorUnsubmitted
Not Done

When I remove this line and switch profile e.g. from Stereo to 5.1 the UI doesn't get updated. I'm no QML expert but it looks like a change to sinkmodel does not result in reevaluation of data(). I'm overriding the binding with an identical one, so I guess it should be fine

nicolasfella: When I remove this line and switch profile e.g. from Stereo to 5.1 the UI doesn't get updated.
droscaUnsubmitted
Not Done

What you do here is assignment to the property, not property binding. So here you actually remove the binding.

Binding would be: grid.pulseObject = Qt.binding(function() { return sinkmodel .... })

In any case, dealing with C++ models outside of delegates is stupid in QML, and QQC1 ComboBox doesn't help.
I've tried to fix it myself and the only thing I came up with is this "brute-force" approach: https://paste.kde.org/po8v22yzp

drosca: What you do here is assignment to the property, not property binding. So here you actually…
121​ onRowsInserted: sinks.updatePulseObject()
122​ onRowsRemoved: sinks.updatePulseObject()
123​ onDataChanged: sinks.updatePulseObject()
124​ }
125​ onCurrentIndexChanged: updatePulseObject()
126​ onCurrentTextChanged: updatePulseObject()
127​ Component.onCompleted: updatePulseObject()
128​
129​ function updatePulseObject() {
droscaUnsubmitted
Not Done

id should be always set as first property.

drosca: id should be always set as first property.
130​ Qt.callLater(function() {
131​ pulseObject = model.data(model.index(sinks.currentIndex, 0), model.role("PulseObject"));
droscaUnsubmitted
Done

No ; at the end of line

drosca: No `;` at the end of line
132​ });
133​ }
134​ }
droscaUnsubmitted
Done
Item {
}
drosca: ```Item { } ```
135​ }
droscaUnsubmitted
Not Done

Make it into one property, without index and role, either with pasting the code inline or moving it to separate function. Also it probably should be readonly property.

drosca: Make it into one property, without `index` and `role`, either with pasting the code inline or…
nicolasfellaAuthorUnsubmitted
Not Done

I couldn't make it readonly because it needs to be updated when the profile is changed. Hence line 117.

nicolasfella: I couldn't make it readonly because it needs to be updated when the profile is changed. Hence…
136​
137​ Grid {
138​ id: grid
139​ columns: 3
140​ spacing: 5
141​ Layout.fillWidth: true
142​
143​ Item {
144​ width: grid.width/3
145​ height: 50
droscaUnsubmitted
Done

remove newline

drosca: remove newline
146​
147​ Button{
148​ text: i18n("Front Left")
149​ anchors.centerIn: parent
150​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Front Left") > -1 : false
151​ onClicked: sinks.pulseObject.testChannel("Front Left")
152​ }
153​ }
154​ Item {
155​ width: grid.width/3
156​ height: 50
157​
158​ Button{
159​ text: i18n("Front Center")
160​ anchors.centerIn: parent
161​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Front Center") > -1 : false
162​ onClicked: sinks.pulseObject.testChannel("Front Center")
163​ }
164​ }
165​ Item {
166​ width: grid.width/3
167​ height: 50
168​
169​ Button{
170​ text: i18n("Front Right")
171​ anchors.centerIn: parent
172​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Front Right") > -1 : false
173​ onClicked: sinks.pulseObject.testChannel("Front Right")
174​ }
175​ }
176​ Item {
177​ width: grid.width/3
178​ height: 50
179​
180​ Button{
181​ text: i18n("Side Left")
182​ anchors.centerIn: parent
droscaUnsubmitted
Done

Indentation

drosca: Indentation
183​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Side Left") > -1 : false
184​ onClicked: sinks.pulseObject.testChannel("Side Left")
185​
186​ }
187​ }
188​ Item {
189​ width: grid.width/3
190​ height: 50
191​
192​ KCoreAddons.KUser {
193​ id: kuser
194​ }
195​
196​ Image {
197​ source: kuser.faceIconUrl
198​ anchors.centerIn: parent
199​ width: 50
200​ height: 50
201​ }
202​ }
203​ Item {
204​ width: grid.width/3
205​ height: 50
206​ Button{
207​ text: i18n("Side Right")
208​ anchors.centerIn: parent
209​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Side Right") > -1 : false
210​ onClicked: sinks.pulseObject.testChannel("Side Right")
211​ }
212​ }
213​ Item {
214​ width: grid.width/3
215​ height: 50
216​ Button{
217​ text: i18n("Rear Left")
218​ anchors.centerIn: parent
219​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Rear Left") > -1 : false
220​ onClicked: sinks.pulseObject.testChannel("Rear Left")
221​ }
222​ }
223​ Item {
224​ width: grid.width/3
225​ height: 50
226​ Button{
227​ text: i18n("Subwoofer")
228​ anchors.centerIn: parent
229​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Subwoofer") > -1 : false
230​ onClicked: sinks.pulseObject.testChannel("Subwoofer")
231​ }
232​ }
233​ Item {
234​ width: grid.width/3
235​ height: 50
236​ Button{
237​ text: i18n("Rear Right")
238​ anchors.centerIn: parent
239​ visible: sinks.pulseObject ? sinks.pulseObject.channels.indexOf("Rear Right") > -1 : false
240​ onClicked: sinks.pulseObject.testChannel("Rear Right")
241​ }
242​ }
243​ }
96 } 244​ }
97} 245​}