D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich accepted this revision.
jgrulich added a comment.
This revision is now accepted and ready to land.


  Looks good to me and also seems to work properly.
  
  Wait for @ngraham if he agrees with this, but I think it's okay in this form 
and it will not bother people by forcing them a search bar when they don't need 
it.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  vpilo/searchBar (branched from master)

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

To: vpilo, #vdg, #plasma, jgrulich
Cc: abetts, jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  In D18607#402657 , @abetts wrote:
  
  > Does the hug allow for just a search button and a non-always visible search 
field?
  
  
  Yep, I want to have it visible only when a user opens it manually, it 
shouldn't be visible by default when the list of connections is long and there 
is a scrollbar visible.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: abetts, jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Andres Betts]

abetts added a comment.


  Does the hug allow for just a search button and a non-always visible search 
field?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: abetts, jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

D18621: Add again a button to manually scan for wireless networks

[Jan Grulich]

jgrulich added a comment.


  In D18621#402605 , @alexeymin 
wrote:
  
  > In D18621#402558 , @ngraham 
wrote:
  >
  > > In my opinion, this sort of thing is a "help the user feel in control" 
issue.
  >
  >
  > +1, it would be cool to have a button to scan for wireless networks. Today 
I tried to connect to Wi-Fi while having a wired connection activated, and no 
W-Fi networks were shown in plasma-nm applet. I had to use `nmtui` to connect 
to wireless net, so shame that plasma-nm lacks this functionality.
  
  
  I will repeat myself, but this shouldn't be needed at all. If you open the 
applet, it immediately triggers new scan, which means that opening the applet 
and clicking on the rescan button will have zero effect, because there is some 
time you need to wait before you can request a new scan. It may take a while 
before your networks appear while the scan is being done.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: alexeymin, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  +1, I actually like it now with the search button. I will try it properly 
later and go through your code.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18621: Add again a button to manually scan for wireless networks

[Valerio Pilo]

vpilo updated this revision to Diff 50583.
vpilo added a comment.


  - Review comments: change button tooltip

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18621?vs=50566=50583

BRANCH
  vpilo/refreshButton (branched from master)

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

AFFECTED FILES
  applet/contents/ui/Toolbar.qml
  libs/handler.cpp
  libs/handler.h

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: alexeymin, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo marked an inline comment as done.
vpilo added inline comments.

INLINE COMMENTS

> PopupDialog.qml:45
>  
> -Toolbar {
> -id: toolbar
> +ColumnLayout {
> +anchors.fill: parent

This is to solve layouting issues when toggling the search bar.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo updated this revision to Diff 50582.
vpilo added a comment.


  - New implementation with a toolbutton
- Also added search reset on Esc keypress, on applet close, on search close
  
  I made a few tests and with all 3 toolbuttons (wifi/mobile/airplane) in 
place, also adding the search bar to the toolbar itself makes it cramped and 
you can't type a lot in it.
  This version IMHO looks neater and I think still pretty unobtrusive, while 
still making the feature discoverable by having a button.

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18607?vs=50567=50582

BRANCH
  vpilo/searchBar (branched from master)

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

AFFECTED FILES
  applet/contents/ui/PopupDialog.qml
  applet/contents/ui/Toolbar.qml
  libs/models/appletproxymodel.cpp

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

[Mélanie Chauvel]

achauvel updated this revision to Diff 50581.

REPOSITORY
  R126 KDE CLI Utilities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15189?vs=50580=50581

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

AFFECTED FILES
  kioclient/kioclient.cpp

To: achauvel, #frameworks, dfaure, cfeck
Cc: plasma-devel, anthonyfieroni, ngraham, kde-frameworks-devel, jraleigh, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

[Mélanie Chauvel]

achauvel updated this revision to Diff 50580.
achauvel added a comment.


  Added an `ifdef` so that it could compile with older version of KIO.

REPOSITORY
  R126 KDE CLI Utilities

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15189?vs=47987=50580

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

AFFECTED FILES
  kioclient/kioclient.cpp

To: achauvel, #frameworks, dfaure, cfeck
Cc: plasma-devel, anthonyfieroni, ngraham, kde-frameworks-devel, jraleigh, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18341: Support icons from local files in buttons

[Nicolas Fella]

nicolasfella added a comment.


  Ping?

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: nicolasfella, #plasma, mart
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

[David Faure]

dfaure added a comment.


#include 

#if KIO_VERSION >= QT_VERSION_CHECK(5,55,0)
...
#endif

REPOSITORY
  R126 KDE CLI Utilities

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

To: achauvel, #frameworks, dfaure, cfeck
Cc: plasma-devel, anthonyfieroni, ngraham, kde-frameworks-devel, jraleigh, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Alexey Min]

alexeymin added a comment.


  In D18621#402558 , @ngraham wrote:
  
  > In my opinion, this sort of thing is a "help the user feel in control" 
issue.
  
  
  +1, it would be cool to have a button to scan for wireless networks. Today I 
tried to connect to Wi-Fi while having a wired connection activated, and no 
W-Fi networks were shown in plasma-nm applet. I had to use `nmtui` to connect 
to wireless net, so shame that plasma-nm lacks this functionality.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: alexeymin, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Valerio Pilo]

vpilo added a comment.


  In D18621#402558 , @ngraham wrote:
  
  > In my opinion, this sort of thing is a "help the user feel in control" 
issue. We had the same sort of conversations in Discover back when it didn't 
have a manual "refresh updates" button. People constantly complained and we had 
to explain over and over again that it automatically fetches when launched and 
periodically after that. Didn't matter; people wanted the button anyway, and 
periodically managed to get themselves into situations where Discover would get 
wedged and a button would actually be nice. Eventually we relented and added 
the button and now people are happy. I think the same thing may be going on 
here and user confidence in the software would benefit. People would feel more 
in control with a button they can click on when it's not doing what they expect 
or not finding their network fast enough.
  
  
  1000% this. Who cares about NM  It really is about making it better for the 
user. I feel that already our 15 seconds timeout is ridiculously long.
  
  I have made some tests with plasma-nm, and then looked in NM's code 
. 
It's an actual hard timeout of 10 whole seconds. 
  I don't really have a clue why would they ever have wanted to introduced this 
long a timeout. Any mobile device, or Windows, can do much faster than this 
crap.
  So, anyways, counting the actual timeout plus whatever time your device takes 
to perform a scan, you can gain up to maybe 4 seconds by refreshing with the 
button.
  
  A plus here: we'd be shifting the blame for updating wifis slowly to the 
actual responsible, networkmanager.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10156: Block geometry updates on move resize finish and don't configure xdg shell surfaces while blocked

[Vlad Zagorodniy]

zzag added a comment.


  We have to send a configure event when the resize is complete. So far it 
looks like this change tries to prevent that; when clientFinishUserMovedResized 
is emitted, geometry updates are still blocked and there is no 
RequestGeometryBlocker around.

REPOSITORY
  R108 KWin

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

To: graesslin, #kwin, #plasma, jgrulich
Cc: zzag, ngraham, plasma-devel, kwin, jraleigh, GB_2, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, alexeymin, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

[Mélanie Chauvel]

achauvel added a comment.


  @dfaure I don’t see how to do that, since there does not seem to have such 
`ifdef` in the actual codebase. However I can make `set(KF5_MIN_VERSION 
"5.55.0")` instead of 5.54.0 in the `CMakeLists.txt`.

REPOSITORY
  R126 KDE CLI Utilities

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

To: achauvel, #frameworks, dfaure, cfeck
Cc: plasma-devel, anthonyfieroni, ngraham, kde-frameworks-devel, jraleigh, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  Besides, it doesn't seem to work as it should:
  
  1. It removed the button in the top right corner which opens the KCM
  2. When I restart plasmashell, the search bar is visible even when I have 
only 4 connections (no scrollbar), typing some text and clearing it makes it 
disappear correctly
  
  Can you look into placing the search bar into the toolbar? Perhaps making it 
visible also only when there are many connections and when it makes sense to 
search for them. I tried to place it there and it looks ugly when you use 
PlasmaComponents.TextField, maybe just some text field without frame and 
borders should be used.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18621: Add again a button to manually scan for wireless networks

[Jan Grulich]

jgrulich added a comment.


  In D18621#402559 , @jgrulich wrote:
  
  > I'm wrong, it actually doesn't perform scanning when the applet is opened. 
It should maybe be doing that, do initial scan on applet popup and then every 
15 seconds only if you keep it opened.
  
  
  Ignore, I was wrong that I was wrong, it does call RequestScan()  when the 
applet is opened.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Jan Grulich]

jgrulich added a comment.


  I'm wrong, it actually doesn't perform scanning when the applet is opened. It 
should maybe be doing that, do initial scan on applet popup and then every 15 
seconds only if you keep it opened.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Nathaniel Graham]

ngraham added a comment.


  In D18621#402540 , @jgrulich wrote:
  
  > I don't think this should be added again, there is no reason to. Plasma-nm 
will do an initial scan once the applet is opened so you get the new networks 
immediately. This has been discussed with NetworkManager developers and 
everyone has to do now periodic scanning in some sane interval. Gnome does the 
some.
  
  
  In my opinion, this sort of thing is a "help the user feel in control" issue. 
We had the same sort of conversations in Discover back when it didn't have a 
manual "refresh updates" button. People constantly complained and we had to 
explain over and over again that it automatically fetches when launched and 
periodically after that. Didn't matter; people wanted the button anyway, and 
periodically managed to get themselves into situations where Discover would get 
wedged and a button would actually be nice. Eventually we relented and added 
the button and now people are happy. I think the same thing may be going on 
here and user confidence in the software would benefit. People would feel more 
in control with a button they can click on when it's not doing what they expect 
or not finding their network fast enough.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Nathaniel Graham]

ngraham added a comment.


  In D18607#402549 , @jgrulich wrote:
  
  >   only when you start typing.
  
  
  Not a great idea since there's no way to know you can search if you start 
typing; it would be an invisible UI and nobody would ever find it. Even in 
GNOME where they have this paradigm everywhere, people hate it because it's not 
discoverable when you want it, and annoying when you accidentally activate it.
  
  I liked your idea of putting a stub in the toolbar that expands to the full 
width when clicked in.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18620: Display shortcut badges while holding Super

[trmdi]

trmdi updated this revision to Diff 50574.
trmdi marked an inline comment as done.
trmdi retitled this revision from "Display shortcut badges while holding 
Super_L" to "Display shortcut badges while holding Super".
trmdi edited the summary of this revision.

REPOSITORY
  R878 Latte Dock

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18620?vs=50569=50574

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

AFFECTED FILES
  CMakeLists.txt
  README.md
  app/CMakeLists.txt
  app/globalshortcuts.cpp
  app/globalshortcuts.h

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  With this approach I will basically see it all the time, you can have just 
one connection hidden and the search bar will pop up, I don't want that and in 
my opinion it doesn't look good. Either should be placed in the toolbar or 
visible only when you start typing.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18620: Display shortcut badges while holding Super_L

[Michail Vourlakos]

mvourlakos added inline comments.

INLINE COMMENTS

> globalshortcuts.cpp:278
> +//display shortcut badges while holding Meta
> +m_mKeyInfoTimer.setInterval(1000);
> +connect(_mKeyInfoTimer, ::timeout, this, [&]() {

I think a lower interval may be better e.g. 500ms

REPOSITORY
  R878 Latte Dock

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

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Jan Grulich]

jgrulich added a comment.


  I don't think this should be added again, there is no reason to. Plasma-nm 
will do an initial scan once the applet is opened so you get the new networks 
immediately. This has been discussed with NetworkManager developers and 
everyone has to do now periodic scanning in some sane interval. Gnome does the 
some.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Nathaniel Graham]

ngraham added a comment.


  +1, this annoys me too.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18620: Display shortcut badges while holding Super_L

[Michail Vourlakos]

mvourlakos added inline comments.

INLINE COMMENTS

> globalshortcuts.cpp:283
> +connect(_keyInfo, ::keyPressed, this, [&](Qt::Key 
> key, bool state) {
> +if (key == Qt::Key_Super_L && state) {
> +m_mKeyInfoTimer.start();

shouldnt this be valid for Key_Super_R also?

REPOSITORY
  R878 Latte Dock

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

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18620: Display shortcut badges while holding Super_L

[trmdi]

trmdi updated this revision to Diff 50569.

REPOSITORY
  R878 Latte Dock

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18620?vs=50564=50569

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

AFFECTED FILES
  CMakeLists.txt
  README.md
  app/CMakeLists.txt
  app/globalshortcuts.cpp
  app/globalshortcuts.h

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18620: Display shortcut badges while holding Super_L

[trmdi]

trmdi added a comment.


  In D18620#402521 , @mvourlakos 
wrote:
  
  > @trmdi
  >
  > 1. does it work? any issues?
  > 2. under wayland did you have any success?
  
  
  
  
  1. It works without any issue with my use case. Not sure other cases. Can you 
test it ?
  2. No, because `KModifierKeyInfo` does not work under wayland: 
https://bugs.kde.org/show_bug.cgi?id=370454
  
  But I guess when it supports wayland, Latte will work too.

REPOSITORY
  R878 Latte Dock

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

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18620: Display shortcut badges while holding Super_L

[Michail Vourlakos]

mvourlakos added a comment.


  @trmdi
  
  1. does it work? any issues?
  2. under wayland did you have any success?

REPOSITORY
  R878 Latte Dock

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

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18620: Display shortcut badges while holding Super_L

[Michail Vourlakos]

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

INLINE COMMENTS

> CMakeLists.txt:57
>  KF5::CoreAddons
> +KF5::GuiAddons
>  KF5::GlobalAccel

please add the new framework to README file also

REPOSITORY
  R878 Latte Dock

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

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo added inline comments.

INLINE COMMENTS

> davidedmundson wrote in PopupDialog.qml:81
> The connection view changes height depending on the presence of a search bar
> The search bar becomes visible depending on the height of the listview
> 
> I would expect that to be giving binding loops when you're on the edge.

How would I test this?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo updated this revision to Diff 50567.
vpilo added a comment.


  - review
  
  Up, not down
  No label
  Changed search label

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18607?vs=50518=50567

BRANCH
  vpilo/searchBar (branched from master)

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

AFFECTED FILES
  applet/contents/ui/PopupDialog.qml
  libs/models/appletproxymodel.cpp

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18621: Add again a button to manually scan for wireless networks

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> Toolbar.qml:124
> +
> +tooltip: i18n("Rescan wireless networks")
> +iconSource: "view-refresh"

How about "Scan for new wireless networks"?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18621: Add again a button to manually scan for wireless networks

[Valerio Pilo]

vpilo created this revision.
vpilo added reviewers: VDG, Plasma, jgrulich, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
vpilo requested review of this revision.

REVISION SUMMARY
  The current 15 seconds timeout is really annoyingly long.
  NetworkManager ignores/refuses most rescan requests when bashing the button,
  that's the reason for the cooldown (preventing flooding NM via dbus).
  
  I also took away the scanning event altogether since it had been removed 
anyways.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  vpilo/refreshButton (branched from master)

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

AFFECTED FILES
  applet/contents/ui/Toolbar.qml
  libs/handler.cpp
  libs/handler.h

To: vpilo, #vdg, #plasma, jgrulich, ngraham
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Nathaniel Graham]

ngraham added a comment.


  BTW regardless of where we put the search field, its placeholder text should 
just be `Search...`.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo added a comment.


  Here's my latest change, not submitted yet.
  
  | F6576947: short.png  | F6576946: 
search.png  | F6576945: long.png 
 |
  |
  
  The differences are that I moved the bar to the top, made the label into a 
placeholder text, and added a bit of margin on the textfield sides.
  
  In D18607#402431 , @jgrulich wrote:
  
  > > Putting the search bar in the toolbar on top is an interesting idea. 
There's definitely room on mine. However it does start to feel a bit cramped. 
+1 for the idea of a search field though.
  >
  > My opinion is: I don't want the search bar to take any additional vertical 
space by default, not even when there is many connections, because I don't need 
usually to scroll down to find the connection I need. On the other hand I get 
it might be useful for someone else. What about showing the search bar once 
users start typing?
  
  
  If you usually don't need to scroll down to find a connection you need, then 
you won't see the search bar.
  
  As you can see:
  
  - When the list of connections is short:
- If the user starts typing, then it shows automatically.
- When the field is emptied, then it hides.
  - When the list of connections is long enough to already need a scrollbar:
- The field is always shown, and highlighted.
- You just lose one entry of vertical space that you have to scroll through 
anyway.
  
  In D18607#402444 , @ngraham wrote:
  
  > In D18607#402435 , @jgrulich 
wrote:
  >
  > > Alternatively we can show a small input box just saying "Search ..."  and 
clicking into it to start typing it would hide all the switches in the toolbar 
(wifi, wwan, airplane) and expand across the whole toolbar?
  >
  >
  > That's not a bad idea! And then when the user clicks on the "clear text" on 
the right side of the search field, it would collapse to its prior size.
  
  
  I think my approach is less visually obtrusive, and you can still use 
everything else in the widget while the search field is active.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18620: Display shortcut badges while holding Super_L

[trmdi]

trmdi created this revision.
trmdi added reviewers: Latte Dock, mvourlakos.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
trmdi requested review of this revision.

REVISION SUMMARY
  Use `KModifierKeyInfo` to display shortcut badges while holding Super_L
  
  BUG: 401768

TEST PLAN
  Just playing with it.
  I still don't understand the `GlobalShortcut` class very well, correct me if 
I make any mistake.

REPOSITORY
  R878 Latte Dock

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

AFFECTED FILES
  CMakeLists.txt
  app/CMakeLists.txt
  app/globalshortcuts.cpp
  app/globalshortcuts.h

To: trmdi, #latte_dock, mvourlakos
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18619: property to override the globaltoolbar style of a single page

[Marco Martin]

mart added a comment.


  F6576894: Screenshot_20190130_171634.png 


REPOSITORY
  R169 Kirigami

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

To: mart, #kirigami, #vdg
Cc: plasma-devel, dkardarakos, apol, davidedmundson, mart, hein

D18619: property to override the globaltoolbar style of a single page

[Marco Martin]

mart added a comment.


  F6576892: Screenshot_20190130_171551.png 


REPOSITORY
  R169 Kirigami

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

To: mart, #kirigami, #vdg
Cc: plasma-devel, dkardarakos, apol, davidedmundson, mart, hein

D18619: property to override the globaltoolbar style of a single page

[Marco Martin]

mart added a comment.


  F6576890: vokoscreen-2019-01-30_17-08-42.mp4 


REPOSITORY
  R169 Kirigami

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

To: mart, #kirigami, #vdg
Cc: plasma-devel, dkardarakos, apol, davidedmundson, mart, hein

D18619: property to override the globaltoolbar style of a single page

[Marco Martin]

mart created this revision.
mart added reviewers: Kirigami, VDG.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
mart requested review of this revision.

REVISION SUMMARY
  add a property in page to override the globaltoolbar style, to either force a 
top
  toolbar on a single page on mobile, or have one single page without any 
toolbar, while keeping 
  it on the others
  
  Also add an optional component proeprty to customize page titles in toolbar 
and titles
  mode
  
  the two features are completely intertwined, so they must come together

TEST PLAN
  added a combobox to try that on kirigami gallery

REPOSITORY
  R169 Kirigami

BRANCH
  mart/titleDelegate

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

AFFECTED FILES
  src/controls/ContextDrawer.qml
  src/controls/Heading.qml
  src/controls/OverlayDrawer.qml
  src/controls/Page.qml
  src/controls/PageRow.qml
  src/controls/private/globaltoolbar/PageRowGlobalToolBarUI.qml
  src/controls/private/globaltoolbar/TitlesPageHeader.qml
  src/controls/private/globaltoolbar/ToolBarPageHeader.qml
  src/controls/templates/OverlayDrawer.qml

To: mart, #kirigami, #vdg
Cc: plasma-devel, dkardarakos, apol, davidedmundson, mart, hein

D18607: Add a popup search bar to the plasma-nm applet

[Nathaniel Graham]

ngraham added a comment.


  In D18607#402435 , @jgrulich wrote:
  
  > Alternatively we can show a small input box just saying "Search ..."  and 
clicking into it to start typing it would hide all the switches in the toolbar 
(wifi, wwan, airplane) and expand across the whole toolbar?
  
  
  That's not a bad idea! And then when the user clicks on the "clear text" on 
the right side of the search field, it would collapse to its prior size.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D17809: Check icon positions after move

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:e2aa89898db3: Check icon positions after move (authored 
by McPain, committed by ngraham).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17809?vs=48238=50560

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

AFFECTED FILES
  containments/desktop/plugins/folder/positioner.cpp

To: McPain, #plasma, hein
Cc: davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  > Putting the search bar in the toolbar on top is an interesting idea. 
There's definitely room on mine. However it does start to feel a bit cramped. 
+1 for the idea of a search field though.
  
  Alternatively we can show a small input box just saying "Search ..."  and 
clicking into it to start typing it would hide all the switches in the toolbar 
(wifi, wwan, airplane) and expand across the whole toolbar?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  In D18607#402415 , @ngraham wrote:
  
  > In D18607#402363 , @jgrulich 
wrote:
  >
  > > I guess it's not, maybe this can be configured in Phabricator somehow, no 
idea, I just don't want to miss any plasma-nm review as its maintainer.
  >
  >
  > This is something that Phabricator can do automatically, if it's 
configured. I've asked Sysadmin to do so: T10420: Plasma Network Management 
Applet herald rule 
  
  
  Great!! Thank you for that.
  
  > Putting the search bar in the toolbar on top is an interesting idea. 
There's definitely room on mine. However it does start to feel a bit cramped. 
+1 for the idea of a search field though.
  
  My opinion is: I don't want the search bar to take any additional vertical 
space by default, not even when there is many connections, because I don't need 
usually to scroll down to find the connection I need. On the other hand I get 
it might be useful for someone else. What about showing the search bar once 
users start typing?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Nathaniel Graham]

ngraham added a comment.


  In D18607#402363 , @jgrulich wrote:
  
  > I guess it's not, maybe this can be configured in Phabricator somehow, no 
idea, I just don't want to miss any plasma-nm review as its maintainer.
  
  
  This is something that Phabricator can do automatically, if it's configured. 
I've asked Sysadmin to do so: T10420: Plasma Network Management Applet herald 
rule 
  
  Putting the search bar in the toolbar on top is an interesting idea. There's 
definitely room on mine. However it does start to feel a bit cramped. +1 for 
the idea of a search field though.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  In D18607#402360 , @vpilo wrote:
  
  > In D18607#402261 , @jgrulich 
wrote:
  >
  > > Wouldn't be a small search bar placed next to the aiplane mode enough? 
That way it will not need additional vertical space. Hiding it when there is 
only a small amount of connections is a good idea.
  >
  >
  > Isn't there way too little space for it inside of the toolbar?
  
  
  Hmm, it probably is, it's not in my case but that depends on screen size and 
resolution. I wouldn't definitely put it at the bottom. To be honest, I 
wouldn't add this at all, because the applet is really small. What I would 
prefer is to add some sort of sorting based on connection priority, that way 
you can have relevant connections at the top and you will not need to search 
for them.
  
  > It would need at least 150-200 px, and that's exactly how much I have in 
that screenshot. In my panel nm applet, there's just a tiny bit more space 
(~50px). How could I visually separate it from the other buttons around it?
  > 
  >> PS: Please add me as a reviewer next time for plasma-nm changes.
  > 
  > Sure. Is this kind of info written anywhere?
  
  I guess it's not, maybe this can be configured in Phabricator somehow, no 
idea, I just don't want to miss any plasma-nm review as its maintainer.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Valerio Pilo]

vpilo added a comment.


  In D18607#402261 , @jgrulich wrote:
  
  > Wouldn't be a small search bar placed next to the aiplane mode enough? That 
way it will not need additional vertical space. Hiding it when there is only a 
small amount of connections is a good idea.
  
  
  Isn't there way too little space for it inside of the toolbar? 
  F6576478: searchbar-searching.png 
  It would need at least 150-200 px, and that's exactly how much I have in that 
screenshot. In my panel nm applet, there's just a tiny bit more space (~50px). 
How could I visually separate it from the other buttons around it?
  
  > PS: Please add me as a reviewer next time for plasma-nm changes.
  
  Sure. Is this kind of info written anywhere?

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18615: Fix mime type filter on background selection button

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Ported to explicit QFileDialog instance instead of static method partly
  because I don't understand how to use the magic nameFilter syntax to set
  mimes, partly to ease the eventual porting to QtQuick which can't have a
  blockign dialog.
  
  BUG: 402930

TEST PLAN
  Clicked on the button
  Only image files were listed
  Selecting one still worked

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  master

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

AFFECTED FILES
  src/configwidgets/selectimagebutton.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18614: Fix loading of background image preview

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  50445f794b3142f29dcc043f3af83c549c694bcd 
 
changed how wallpapers worked.
  Instead of storing an absolute path in the config we move the file and
  saves a relative path.
  
  The preview icon still expected an absolute path.
  
  This patch loads the image in the same way SDDM expects.
  
  BUG: 402930
  FIXED-IN: 5.15.0

TEST PLAN
  Set image
  Reopened KCM

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  master

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

AFFECTED FILES
  src/themeconfig.cpp
  src/themeconfig.h

To: davidedmundson, #plasma
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:aaebb51077ae: Defer initial positions apply until listing 
is complete (authored by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D18598?vs=50509=50545#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50509=50545

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17706: Fix lock screen focus

[Andrey Bychkov]

andreyby added a comment.


  On x11 flag removal does not affect the created window, we can use this to 
put the focus.

INLINE COMMENTS

> graesslin wrote in greeterapp.cpp:460
> David already said it: we cannot do this. Changing the 
> X11BypassWindowManagerHint flag after the window is created is not supported 
> by X11 protocol! We really, really cannot do this.

I understand, but only without X11BypassWindowManagerHint flag set can the 
focus be set correctly

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

To: andreyby, davidedmundson
Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

D17706: Fix lock screen focus

[Andrey Bychkov]

andreyby updated this revision to Diff 50543.
andreyby added a comment.


  It's works for x11 platform.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17706?vs=49444=50543

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

AFFECTED FILES
  greeter/greeterapp.cpp

To: andreyby, davidedmundson
Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart

D18607: Add a popup search bar to the plasma-nm applet

[Jan Grulich]

jgrulich added a comment.


  Wouldn't be a small search bar placed next to the aiplane mode enough? That 
way it will not need additional vertical space. Hiding it when there is only a 
small amount of connections is a good idea.
  
  PS: Please add me as a reviewer next time for plasma-nm changes.

REPOSITORY
  R116 Plasma Network Management Applet

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

To: vpilo, #vdg, #plasma
Cc: jgrulich, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D18578: Reduce string objects

[Tomaz Canabrava]

tcanabrava added a comment.


  In D18578#401842 , @apol wrote:
  
  > Looks okay from a theoretical point of view, I can expect the practical 
impact of this be about 0~ :P
  
  
  What I really like about math is that sum of changes with  zero impact i 
never zero :D

REPOSITORY
  R106 KSysguard

BRANCH
  reduceStrings

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

To: tcanabrava, apol
Cc: apol, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, mart

D18579: Rework Sensor Browser

[Tomaz Canabrava]

tcanabrava added inline comments.

INLINE COMMENTS

> argonel wrote in SensorBrowser.cpp:51
> This one caused me to notice the style issues. The rule is "For pointers or 
> references, use a single space before '*' or '&', but not after"

ups. that was a typo.

> argonel wrote in SensorBrowser.cpp:624
> The removal of this constructor eliminates this assert. You've kept the 
> others, perhaps move it to SensorBrowserModel::makeSensor?

I could argue that's impossible to hit this assert and that's preciosism (just 
like a try { } catch (...) after every 'new') but I can do that.

REPOSITORY
  R106 KSysguard

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

To: tcanabrava
Cc: argonel, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart