D13931: [KCM] Add speaker placement test

[David Rosca]

drosca requested changes to this revision.
drosca added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> Advanced.qml:112
> +
> +            delegate:
> +

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 {
      // ...
  }

> Advanced.qml:131
> +                    }
> +                    Item{
> +

Item {
  }

> sink.cpp:89
> +{
> +    if (name == QStringLiteral("Front Left"))
> +        return PA_CHANNEL_POSITION_FRONT_LEFT;

if (name == QLatin1String("Front Left")) {
      // ...
  } else if (...) {
      //
  }

> sink.cpp:118
> +{
> +    switch (position) {
> +        case PA_CHANNEL_POSITION_FRONT_LEFT:

switch (position) {
  case ...:
  case ...:
  }

> sink.h:48
> +public slots:
> +    void testChannel(const QString& name);
> +

`const QString &name`

> sink.h:55
> +
> +    ca_context *m_canberra;
> +

It should reuse context from `VolumeFeedback`.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D13931

To: nicolasfella, drosca
Cc: ngraham, #vdg, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180708/8abec2e7/attachment.html>