Differential D27074

[Applet]Add port check
ClosedPublic
Actions

Authored by gvgeo on Jan 31 2020, 4:51 PM.

Details

Reviewers
drosca
ngraham
Group Reviewers
Plasma
Commits
R115:8f0847af1d17: [Applet]Add port check
Summary

Add port check before port.description check.

Cannot check port.description if port does not exist.

Test Plan

Before:
If there is a device with ports, and an other one without ports, will fail to display the name a device, or crash?

After:
Name should be displayed correctly in every case.

Diff Detail

Repository
R115 Plasma Audio Volume Applet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
gvgeo created this revision.Jan 31 2020, 4:51 PM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptJan 31 2020, 4:51 PM
gvgeo requested review of this revision.Jan 31 2020, 4:51 PM
ngraham added a comment.Jan 31 2020, 4:53 PM

What issue is this fixing? Can you update the description and test plan sections?

gvgeo planned changes to this revision.Jan 31 2020, 5:00 PM
gvgeo edited the summary of this revision. (Show Details)
gvgeo edited the test plan for this revision. (Show Details)
ngraham added a comment.Jan 31 2020, 5:03 PM

Thanks!

Are you planning code changes? Or was that "Plan changes" just about updating the description and test plan?

gvgeo retitled this revision from [Applet]Fix port check to [Applet]Add port check.Jan 31 2020, 5:03 PM
gvgeo edited the summary of this revision. (Show Details)

Give 5 min to update the code.

gvgeo updated this revision to Diff 74811.Jan 31 2020, 5:14 PM

Added port.description too.
There is port.description.length now, better safe than sorry.

ngraham added a comment.Jan 31 2020, 5:17 PM

Maybe only show the description when there's more than one input or output device? I thought the lack of a description for the single-device use case was a feature! :) It looks so nice and clean:

gvgeo added a comment.Jan 31 2020, 5:27 PM
In D27074#604235, @ngraham wrote:

Maybe only show the description when there's more than one input or output device? I thought the lack of a description for the single-device use case was a feature! :) It looks so nice and clean:

II don't see the problem, in this picture. I don't even know if it is before of after this patch.
It looks fine, both devices show the port.

This should not change anything. It is a bug fix.
There are 2 descriptions, Device description and Port description. We prefer to show the Port description.

ngraham accepted this revision.Jan 31 2020, 6:08 PM

Oh you're right, I was looking at the wrong thing.

This revision is now accepted and ready to land.Jan 31 2020, 6:08 PM
Closed by commit R115:8f0847af1d17: [Applet]Add port check (authored by gvgeo, committed by ngraham). · Explain WhyJan 31 2020, 6:09 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 74812

View Options

applet/contents/ui/DeviceListItem.qml

Loading...