D26271: [Applet]Update layout based on T10470

gvgeo added a comment. Change to implicitHeight in D27117 . That line should be in D26574 , messed up splitting the patches. It keeps the height of the row fixed, so that it won't change when the button gets

D26271: [Applet]Update layout based on T10470

davidedmundson added inline comments. INLINE COMMENTS > ListItemBase.qml:111 > RowLayout { > -Layout.fillWidth: true > +Layout.minimumHeight: contextMenuButton.height > That's asking for a binding loop. An implicit size should never be

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. In D26271#604203 , @ngraham wrote: > @gvgeo though I tested this before and found it to be working, after landing the patch I'm now seeing that using the new radio buttons to change the default device does not

D26271: [Applet]Update layout based on T10470

ngraham added a comment. @gvgeo though I tested this before and found it to be working, after landing the patch I'm now seeing that using the new radio buttons to change the default device does not automatically move all running streams to the new device, as it used to. Can you check this?

D26271: [Applet]Update layout based on T10470

This revision was automatically updated to reflect the committed changes. Closed by commit R115:8ada15ad8e5b: [Applet]Update layout based on T10470 (authored by gvgeo, committed by ngraham). REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE

D26271: [Applet]Update layout based on T10470

ngraham added a comment. I applied the patches in this series and tested and went through the code again and everything looks good to me. Shall we start landing them for 5.19? We can always tweak things later; there's loads of time before the next release. REPOSITORY R115 Plasma Audio

D26271: [Applet]Update layout based on T10470

manueljlin accepted this revision. manueljlin added a comment. Nice, looks great! About the stream name, it doesn't look like it's useful most of the time so it might be better to remove it REPOSITORY R115 Plasma Audio Volume Applet BRANCH layout2 (branched from master) REVISION DETAIL

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 74145. gvgeo added a comment. Correct update. Last was wrong one. REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=74141=74145 BRANCH layout2 (branched from master) REVISION DETAIL

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 74141. gvgeo added a comment. Update for latest scrollbar fixes (remove workaround). REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=73919=74141 BRANCH menu3 (branched from master) REVISION

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 73919. gvgeo added a comment. rebase REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=73238=73919 BRANCH layout2 (branched from master) REVISION DETAIL https://phabricator.kde.org/D26271

D26271: [Applet]Update layout based on T10470

cfeck added a comment. The radio stream I use (via mpv) shows the interpret and title information for all titles it plays. Also, when mpv plays a local file, ttitle information about the file is added there. I don't use any other media streams, so I cannot comment on the general

D26271: [Applet]Update layout based on T10470

ndavis added a subscriber: cfeck. ndavis added a comment. Since it's @cfeck's bug report, I'll add him to the subscribers. REPOSITORY R115 Plasma Audio Volume Applet BRANCH layout (branched from master) REVISION DETAIL https://phabricator.kde.org/D26271 To: gvgeo, #vdg, #plasma,

D26271: [Applet]Update layout based on T10470

ndavis added a comment. In D26271#593013 , @gvgeo wrote: > Found this request to display the name of the stream https://bugs.kde.org/show_bug.cgi?id=409453 . > Which was done, but remove here. Maybe should leave it too? > With a quick

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. Found this request to display the name of the stream https://bugs.kde.org/show_bug.cgi?id=409453 . Which was done, but remove here. Maybe should leave it too? With a quick check, not many applications have this information. F7880660: Screenshot_20200113_135516.png

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. Can't include an untested patch because we are in a hurry. To clarify my comment's D26574#591702 part "If these patches ever get accepted". I wasn't aware of the release schedule, and my doubts are about code

D26271: [Applet]Update layout based on T10470

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. I'm good with this now as an initial implementation of the mockup. However we're a bit close to the Plasma 5.18 tagging date (January 16th). My gut instinct is to land these early in the

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 73238. gvgeo added a comment. Separate bug fixes. Change defaultButton visibility Change defaultButton's RowLayout height REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=72914=73238 BRANCH

D26271: [Applet]Update layout based on T10470

ngraham added a comment. Yeah, feel free to fix those bugs separately so we can get them in more quickly. Personally I don't think there's a major use case for manipulating the settings of a port when it's not actually being used. REPOSITORY R115 Plasma Audio Volume Applet REVISION

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. It makes possible to change volume/mute before plug something. But have no idea, if there is any real reason. --- About these 2 bugs https://bugs.kde.org/show_bug.cgi?id=393418, https://bugs.kde.org/show_bug.cgi?id=413448 . Should I separate the fixes from

D26271: [Applet]Update layout based on T10470

ngraham added a comment. > But this would made impossible to select an unavailable port from kcm too. Is there an actual use case for this? If not, option 2 might make the most sense. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D26271 To:

D26271: [Applet]Update layout based on T10470

gvgeo planned changes to this revision. gvgeo added a comment. Found a bug. There is no update when a port that is not in use gets unavailable. Why is a problem in this patch: When using port 1, if port 2 becomes unavailable, is possible to switch to port that is unavailable. The

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72914. gvgeo added a comment. Add description check REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=72854=72914 BRANCH layout (branched from master) REVISION DETAIL

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. In D26271#588909 , @ngraham wrote: > Lovely. Any chance we could make the bottom divider touch the edges of the window? Not really, not from here. This space is part of system tray. If you use the plasma-pa widget

D26271: [Applet]Update layout based on T10470

ngraham added a comment. Lovely. Any chance we could make the bottom divider touch the edges of the window? I suppose we'll need to adjust the Breeze Plasma theme for the changes to the tab bar appearance. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL

D26271: [Applet]Update layout based on T10470

gvgeo added inline comments. INLINE COMMENTS > DeviceListItem.qml:36 > +var name = model.data(model.index(i, 0), > model.role("Ports")) > +[model.data(model.index(i, 0), > model.role("ActivePortIndex"))].description; > +

D26271: [Applet]Update layout based on T10470

ndavis added a comment. In D26271#588570 , @gvgeo wrote: > It is a theme problem. Breeze light also has problem. > Widgets take color from Plasma Style. While in applications are fine. > I checked with this widget

D26271: [Applet]Update layout based on T10470

manueljlin added a comment. If it isn't possible that's okay (it looks really nice right now, great job btw :D), although it'd be nice to make it consistent with the kirigami style divider REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D26271

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. It is a theme problem. Breeze light also has problem. Widgets take color from Plasma Style. While in applications are fine. I checked with this widget to compare. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL

D26271: [Applet]Update layout based on T10470

ndavis added a comment. Why do the unchecked radiobuttons have a dark outline in your screenshot? REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D26271 To: gvgeo, #vdg, #plasma, manueljlin, drosca, ngraham Cc: ndavis, filipf, ngraham,

D26271: [Applet]Update layout based on T10470

gvgeo marked 5 inline comments as done. gvgeo added a comment. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D26271 To: gvgeo, #vdg, #plasma, manueljlin, drosca, ngraham Cc: filipf, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72854. gvgeo added a comment. leanup Remove separation lines Fix 2 bugs Full Name display when a port name sub-strings are same. Requested changes Issues: 1. Check comment at the bottom of main.qml in D26256

D26271: [Applet]Update layout based on T10470

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Very nice work! I continue to be impressed with your contributions. > What else to do, without displaying the full device names? Display full device names when there's

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72750. gvgeo added a comment. Add visibility to the line that splits playback and record. On the application side, will show if there is 1 record and 1 playback application. Because there is no port name to easily differentiate them. REPOSITORY R115

D26271: [Applet]Update layout based on T10470

gvgeo marked 7 inline comments as done. gvgeo added a comment. About the devices labels, I feel it is fine as it is. It is not a problem, to have two devices with the same port name. Only if could put a tooltip. But no, Radiobutton does not deserve a tooltip. What else to do, without

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72739. gvgeo added a comment. Remove port combobox and put port selection back the hamburger menu. Now port selection appears only if there is at least 2 available ports. Made devices/applications labels clickable. Removed RTL margin problems.

D26271: [Applet]Update layout based on T10470

filipf added inline comments. INLINE COMMENTS > ListItemBase.qml:117 > +id: defaultButton > +Layout.leftMargin: units.smallSpacing * 0.75 > +checked: PulseObject.default Margins should but don't get mirrored when using a

D26271: [Applet]Update layout based on T10470

ngraham added a comment. Without device icons, the text needs to be improved a bit, or else I see this when I have multiple devices: F7851224: Screenshot_20191230_145819.png INLINE COMMENTS > ListItemBase.qml:115 > > +

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72373. gvgeo added a comment. Brought back spacing, and corrected sizes. Centered icons, and align text. Reduced checkbox to 18 pixel. QQC2 checkbox is 16 pixels, but does not scale. Instead used PlasmaComponents3 which is 18. REPOSITORY R115

D26271: [Applet]Update layout based on T10470

ngraham added a comment. Awesome work, @gvego. I'm really liking all of these patches of yours. :) The checkbox size issue is probably from the styling of the PlasmaComponents checkbox, as opposed to the basic QQC2 Checkbox. If I'm right, we'd need to fix the PC checkbox, or else just

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. I was not able to change the checkbox size, nothing I tried worked. Checked other places with smaller checkbox(like notifications). But could not found how the size is controlled. Current view: F7849371: Screenshot_20191229_220847.png

D26271: [Applet]Update layout based on T10470

gvgeo updated this revision to Diff 72345. gvgeo added a comment. Fix spacing Change plasmoid tooltip to mirror the port name Cleanup main.qml REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26271?vs=72321=72345 BRANCH UI (branched

D26271: [Applet]Update layout based on T10470

manueljlin added a comment. Looking at the screenshots, UI wise it's looking good already, only that the checkbox seems too big (should be the same size as the radio button, 16px) and there's a 5px sized padding after the divider that cuts off part of the Internal Microphone button and

D26271: [Applet]Update layout based on T10470

gvgeo added a comment. Looks weird right now. From what I can tell, themes add lines between list items. There must be some easy way to disable it. F7848489: Screenshot_20191229_114632.png REPOSITORY R115 Plasma Audio Volume Applet REVISION

D26271: [Applet]Update layout based on T10470

gvgeo created this revision. gvgeo added reviewers: VDG, Plasma, manueljlin, drosca. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. gvgeo requested review of this revision. REVISION SUMMARY Change layout according to T10470