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

2019-01-31 Thread Valerio Pilo
This revision was automatically updated to reflect the committed changes. Closed by commit R116:684ee0c13bfb: Add a popup search bar to the plasma-nm applet (authored by vpilo). REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE

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

2019-01-31 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. Yep, I think this is fine. Shipit! REPOSITORY R116 Plasma Network Management Applet BRANCH vpilo/searchBar (branched from master) REVISION DETAIL https://phabricator.kde.org/D18607 To: vpilo, #vdg, #plasma, jgrulich, ngraham

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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,

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

2019-01-30 Thread 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,

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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.

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

2019-01-30 Thread 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.

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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,

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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 ..."

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-30 Thread 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

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

2019-01-29 Thread Nathaniel Graham
ngraham added a comment. In D18607#402168 , @vpilo wrote: > Better just place it at the top above the the connections list, then? I think that would make sense. In such a limited space, I think it's fine for the search field to only

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

2019-01-29 Thread Valerio Pilo
vpilo added a comment. Better just place it at the top above the the connections list, then? REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D18607 To: vpilo, #vdg, #plasma Cc: davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel,

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

2019-01-29 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > davidedmundson wrote in appletproxymodel.cpp:60 > what happens if you have a filter and the the entry is a slave? Edit: ignore this. REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D18607

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

2019-01-29 Thread David Edmundson
davidedmundson added a comment. Seems niche, but this solution is also unobtrusive, so I like that. I have a lot of VPNs searching through them would be quite useful. Having the search bar on the bottom is a bit weird as when you filter the results will be at the top, meaning the user's

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

2019-01-29 Thread Valerio Pilo
vpilo created this revision. vpilo added reviewers: VDG, Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. vpilo requested review of this revision. REVISION SUMMARY If there are not enough connections to fill the scrollview, the search bar is not displayed by