D13481: Recommend window border size "None"

2018-06-18 Thread Nathaniel Graham
ngraham added a comment.


  Does this/should this also implement the diff from D13278 
?

REPOSITORY
  R31 Breeze

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

To: romangg, #plasma, #vdg
Cc: ngraham, davidedmundson, graesslin, abetts, mart, plasma-devel, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 36300.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13529?vs=36270=36300

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

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


D13596: Fix there being more security updates than total updates in notifier

2018-06-18 Thread Nathaniel Graham
ngraham added subscribers: apol, ngraham.
ngraham added reviewers: Discover Software Store, apol.
ngraham added a comment.


  For bugfixes like this, the workflow is to commit to the "stable branch" (in 
this case `Plasma/5.12`) and then merge to master. In this case you would also 
merge to `Plasma/5.13`. the `FIXED-IN` tag should be `5.12.6`.
  
  See also 
https://community.kde.org/Infrastructure/Phabricator#Landing_on_the_.22Stable_branch.22
  
  As for the patch, I tried it out and it seems to solve the problem! No 
opinion on whether your current approach or the alternative one you mentioned 
would be most appropriate. I'll let @apol have the final say.

REPOSITORY
  R134 Discover Software Store

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

To: Zren, #discover_software_store, apol
Cc: ngraham, apol, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, mart


D13596: Fix there being more security updates than total updates in notifier

2018-06-18 Thread Chris Holland
Zren created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
Zren requested review of this revision.

REVISION SUMMARY
  We currently read the previous `m_securityCount` instead of using the current 
`securityCount`.
  
  BUG: 392056
  FIXED-IN 5.13.1 (or 5.13.2?)
  FIXED-IN 5.12.6 (This should be backported right?) 
https://github.com/KDE/discover/blame/Plasma/5.12/notifier/DiscoverNotifier.cpp#L82
  
  The bug report is from 5.12.3, and was also reported on reddit today. Both 
have a screenshot of the bug.
  https://bugs.kde.org/show_bug.cgi?id=392056
  
https://www.reddit.com/r/kde/comments/8rzrr6/so_whats_the_logic_behind_updater/
  
  We could also start the counter at 0 and add `securityCount` afterward.
  
-uint count = securityUpdatesCount();
+uint count = 0;
 foreach(BackendNotifierModule* module, m_backends)
 count += module->updatesCount();
+count += securityCount;

TEST PLAN
  I haven't attempted to compile this or test it. I don't have any security 
updates atm anyways.
  I haven't pushed a bugfix to a `Plasma/5.__` branch before either. Do I make 
a separate commit for 5.12, 5.13, and master?

REPOSITORY
  R134 Discover Software Store

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

AFFECTED FILES
  notifier/DiscoverNotifier.cpp

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


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread David Edmundson
davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> davidedmundson wrote in main.qml:98
> Oh!
> 
> The old c++ plugin had deactivate(bool) where the parameter indicated whether 
> we restored the minimised windows or not. I had completely misread the old 
> behaviour.
> 
>   onVirtualDesktopChanged: deactivate()
>   onActivityChanged: deactivate()
> 
> should be doing what you're doing here not calling deactivate().
> 
> Can you change those, then  to that specific change.
> (BUG: 395519)
> 
> ---
> 
> as for onDataChanged vs onActiveTaskChanged
> 
> The reason I did this was if you click minimise all -> then click on the 
> desktop we deactivate, something I don't think we want.
> 
> I don't fully understand what you're saying is wrong with the onDataChanged.

this is deactivating the minimise all whenever the IsWindow role of any window 
changes? That seems wrong.

Though between the two patches we might have a good idea.

We could use onActiveTaskChanged if we then check that the activeTask isWindow 
is true.

REPOSITORY
  R114 Plasma Addons

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

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


D13095: Scren brightness follow a quatratic progression

2018-06-18 Thread Scott Harvey
sharvey added a comment.


  A couple of random ideas:
  
  - The Arch Wiki has a lengthy page 
 on screen brightness. Maybe 
there's something there that can help.
  - Is there a valid use case for going all the way to zero? Minimum doesn't 
necessarily need to equal "off". If it's an external screen, it's got a power 
button. The only complex situation I can imagine is a laptop + monitor dual 
screen setup. Thoughts?
  - Perhaps, when starting from zero, move two steps instead of just one. That 
would fix the "Nate Problem", and i doubt anyone would say "okay, my screen is 
really dim, but I wanted it even dimmer."

REPOSITORY
  R122 Powerdevil

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

To: thsurrel, #plasma, broulik, ngraham
Cc: sharvey, zzag, ngraham, romangg, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13095: Scren brightness follow a quatratic progression

2018-06-18 Thread Nathaniel Graham
ngraham added a comment.


  I don't think we want the possibility of breaking anyone. A setting doesn't 
seem ideal either; who would ever find and change this?
  
  Rather, let's focus on the issue that's hurting us here to see if we can 
resolve it somehow. Is there any way to programmatically detect the minimum 
brightness before the backlight turns off? Or could it be a kernel bug that 
there are ever any non-zero brightness values that still have the backlight off?

REPOSITORY
  R122 Powerdevil

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

To: thsurrel, #plasma, broulik, ngraham
Cc: sharvey, zzag, ngraham, romangg, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13095: Scren brightness follow a quatratic progression

2018-06-18 Thread Thomas Surrel
thsurrel added a comment.


  > Same values as `/sys/class/backlight/*/brightness`, in other words.
  
  Thanks Nate, even if that's not the answer I was hoping for ...
  The future of this patch:
  
  - accept to break some systems (but who wants that ?)
  - add an option for this quadratic progression somehwere
  - drop it
  
  What do you think ? (the question is for anybody interested!)

REPOSITORY
  R122 Powerdevil

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

To: thsurrel, #plasma, broulik, ngraham
Cc: sharvey, zzag, ngraham, romangg, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D6096: Add Wayland RemoteAccess capabilities to KRfb

2018-06-18 Thread Vlad Zagorodniy
zzag added inline comments.

INLINE COMMENTS

> pw_framebuffer.h:27
> +PWFrameBuffer(WId winid, QObject *parent = nullptr);
> +virtual ~PWFrameBuffer() override;
> +

`virtual` is redundant. (also, it doesn't make sense to have both `virtual` and 
`override`)

> pw_framebufferplugin.h:36
> +   PWFrameBufferPlugin(QObject *parent, const QVariantList );
> +   virtual ~PWFrameBufferPlugin() override;
> +

`virtual` is redundant.

REPOSITORY
  R437 Desktop Sharing

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

To: Kanedias, davidedmundson, graesslin, #plasma, #kde_applications
Cc: zzag, jgrulich, alexeymin, plasma-devel, ragreen, Pitel, schernikov, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, 
mart, hein


D13593: [Fonts KCM] Improve user-friendliness of some anti-aliasing strings

2018-06-18 Thread Nathaniel Graham
ngraham added a comment.


  Most of this stuff used to be hidden behind a Configure button, but it was 
changed to put everything on the main page in 5.13 as part of the QML port: 
D8916 
  
  IMHO as long as we're going to present everything on the main page, we need 
to make sure it's as user-friendly and comprehensible as possible. I think the 
sort of person who knows what "anti-aliasing" means is likely to understand 
that "font smoothing" is the same thing. I'm less sure that the reverse is 
true, which is why I submitted this patch (and presumably why @abetts filed the 
bug).

REPOSITORY
  R119 Plasma Desktop

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

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


D13593: [Fonts KCM] Improve user-friendliness of some anti-aliasing strings

2018-06-18 Thread Andres Betts
abetts added a comment.


  In D13593#279642 , @nicolasfella 
wrote:
  
  > Sometimes it's better to be precise about well-known technical terms 
instead of hiding them. Would a person who does not know what anti-aliasing is 
want/need to change that setting?
  
  
  I agree, this is a very fundamental question we should always make as we 
design. We have our motto, simple by default, powerful when needed. I feel this 
setting is more "powerful when needed." I would think that it needs to be in an 
'advanced' section or in second place to fundamental controls. You can decide 
how to present it.

REPOSITORY
  R119 Plasma Desktop

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

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


D13593: [Fonts KCM] Improve user-friendliness of some anti-aliasing strings

2018-06-18 Thread Nicolas Fella
nicolasfella added a comment.


  Sometimes it's better to be precise about well-known technical terms instead 
of hiding them. Would a person who does not know what anti-aliasing is 
want/need to change that setting?

REPOSITORY
  R119 Plasma Desktop

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

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


D13593: [Fonts KCM] Improve user-friendliness of some anti-aliasing strings

2018-06-18 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Plasma, VDG.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Improve the user-friendliness of some strings in the Fonts KCM. 
"Anti-aliasing" is a
  technical term not well understood by people who aren't like us, and we can 
also shorten
  some strings with no loss of meaning.
  
  BUG: 386566
  FIXED-IN 5.14

TEST PLAN
  Before:
  
  After:

REPOSITORY
  R119 Plasma Desktop

BRANCH
  more-user-friendly-strings-in-fonts-KCM (branched from master)

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

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

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


D13591: Set complete vectors instead of creting them at runtime

2018-06-18 Thread Vlad Zagorodniy
zzag added inline comments.

INLINE COMMENTS

> zzag wrote in breezesizegrip.cpp:150
> Can be const.

Also, couldn't you do

  painter.drawPolygon( QVector<>{}
  );

REPOSITORY
  R31 Breeze

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

To: tcanabrava, #breeze
Cc: ngraham, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13591: Set complete vectors instead of creting them at runtime

2018-06-18 Thread Tomaz Canabrava
tcanabrava added inline comments.

INLINE COMMENTS

> zzag wrote in breezeshadowhelper.cpp:461-464
> Not the best thing. But shadows in Breeze have positive margins so that's 
> okay.

they where added to the data vector before, and the data vector is quint32, so 
I don't see the difference.

REPOSITORY
  R31 Breeze

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

To: tcanabrava, #breeze
Cc: ngraham, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13591: Set complete vectors instead of creting them at runtime

2018-06-18 Thread Vlad Zagorodniy
zzag added inline comments.

INLINE COMMENTS

> breezesizegrip.cpp:54
>  // mask
> -QPolygon p;
> -p << QPoint( 0, GripSize )
> -<< QPoint( GripSize, 0 )
> -<< QPoint( GripSize, GripSize )
> -<< QPoint( 0, GripSize );
> +QPolygon p = QVector{
> +QPoint( 0, GripSize ),

Can be const.

> breezesizegrip.cpp:150
>  // polygon
> -QPolygon p;
> -p << QPoint( 0, GripSize )
> -<< QPoint( GripSize, 0 )
> -<< QPoint( GripSize, GripSize )
> -<< QPoint( 0, GripSize );
> +QPolygon p = QVector {
> +QPoint( 0, GripSize ),

Can be const.

> breezeshadowhelper.cpp:461-464
> +const quint32 topSize = margins.top();
> +const quint32 bottomSize = margins.bottom();
> +const quint32 leftSize( margins.left() );
> +const quint32 rightSize( margins.right() );

Not the best thing. But shadows in Breeze have positive margins so that's okay.

REPOSITORY
  R31 Breeze

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

To: tcanabrava, #breeze
Cc: ngraham, zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13591: Set complete vectors instead of creting them at runtime

2018-06-18 Thread Vlad Zagorodniy
zzag added a comment.


  Maybe, it makes sense to create a static const, e.g.
  
static const QVector s_minimizeButtonPoly {
QPointF( 4, 9 ),
QPointF( 9, 4 ),
QPointF( 14, 9 ),
QPointF( 9, 14 )
};

REPOSITORY
  R31 Breeze

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

To: tcanabrava, #breeze
Cc: zzag, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D13573: Touchpad KCM QtQuickControls2 Conversion

2018-06-18 Thread Furkan Tokac
furkantokac added a comment.


  @davidedmundson @mart 
  Thanks for the feedback. New patch is coming.

REPOSITORY
  R119 Plasma Desktop

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

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


D13591: Set complete vectors instead of creting them at runtime

2018-06-18 Thread Tomaz Canabrava
tcanabrava created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
tcanabrava requested review of this revision.

REVISION SUMMARY
  Don't iterate over a vector to append in a vector
  
  Append a vector instead of 4 items individually
  
  Speedup size grip creation
  
  Speedup drawing of Breeze Buttons

REPOSITORY
  R31 Breeze

BRANCH
  removeTemporaryReallocs

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

AFFECTED FILES
  kdecoration/breezebutton.cpp
  kdecoration/breezesizegrip.cpp
  kstyle/breezehelper.cpp
  kstyle/breezeshadowhelper.cpp

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


D13481: Recommend window border size "None"

2018-06-18 Thread David Edmundson
davidedmundson added a comment.


  There are two visual hints when the mouse is inside the deadzone.
  
  You would still get the different cursor
   The client gets the mouse left event and would not show any hover effect on 
whatever control might be there.

REPOSITORY
  R31 Breeze

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

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


D13481: Recommend window border size "None"

2018-06-18 Thread Martin Flöser
graesslin added a comment.


  I realized that there's yet another problem with the approach: if two windows 
border and the window with pointer focus is lower in the stacking order this 
would create a dead zone in the window. With compositing disabled this would be 
worse as there's not even a visual hint.
  
  I really need to urge vdg to rethink all of that. I have a really bad feeling 
about turning no borders on by default. Please reconsider.

REPOSITORY
  R31 Breeze

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

To: romangg, #plasma, #vdg
Cc: graesslin, abetts, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol


D13481: Recommend window border size "None"

2018-06-18 Thread Andres Betts
abetts added a comment.


  +1

REPOSITORY
  R31 Breeze

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

To: romangg, #plasma, #vdg
Cc: abetts, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Fabian Vogt
fvogt added a comment.


  In D13586#279514 , @ngraham wrote:
  
  > Could this have any relationship to 
https://bugs.kde.org/show_bug.cgi?id=395401?
  
  
  In theory yes, this is undefined behaviour and could cause everything between 
making a pixel slightly darker or crashing the harddrive.
  
  In practice I doubt it - this doesn't touch any of the code related to saving.

REPOSITORY
  R119 Plasma Desktop

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

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


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Nathaniel Graham
ngraham added a comment.


  Could this have any relationship to 
https://bugs.kde.org/show_bug.cgi?id=395401?

REPOSITORY
  R119 Plasma Desktop

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

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


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Fabian Vogt
fvogt added a comment.


  I guess KSharedConfig would work as well, that way the KConfig keeps a 
reference alive.

REPOSITORY
  R119 Plasma Desktop

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

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


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread David Edmundson
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  In current code we would have a KConfigGroup with a dangling KConfig
  deleted after the RHS for the group fetch has finished.
  
  BUG: 394534

TEST PLAN
  Wrote minimal test case of code
  It produced a valgrind warning (weirdly didn't crash though)
  Modified to correct version
  No longer any warnings

REPOSITORY
  R119 Plasma Desktop

BRANCH
  davidedmundson/kcm_clock_qml

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

AFFECTED FILES
  kcms/input/backends/x11/x11_backend.cpp

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


D13481: Recommend window border size "None"

2018-06-18 Thread Marco Martin
mart added a comment.


  +1

REPOSITORY
  R31 Breeze

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

To: romangg, #plasma, #vdg
Cc: mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread Anthony Fieroni
anthonyfieroni updated this revision to Diff 36270.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13529?vs=36160=36270

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

AFFECTED FILES
  applets/minimizeall/package/contents/ui/main.qml

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


D13573: Touchpad KCM QtQuickControls2 Conversion

2018-06-18 Thread Marco Martin
mart requested changes to this revision.
mart added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> davidedmundson wrote in main.qml:23
> this is either unused or wrong

he's using qqc1 tooltips, they should be ported as well.

> davidedmundson wrote in main.qml:37
> Saying the size hint is the size it currently is looks wrong
> 
> generally speaking:
> 
> width propagates from parent to child
>  implicitWidth propagates from child to parent

and what those sizehint properties are used for at all? they look wrong,
you either have implicitwidth/height or Layout.minimum/preferred/maximumwidth 
and height.
implementing yourself hints should generally never be done as it will break 
very easily

> main.qml:285
> +
> +ToolTip {
> +text: i18n("Cursor moves the same distance as finger.")

i guess this is the thing that comes from styles 1.4
there is also a qqc2 ToolTip element, which is the one that should be used

REPOSITORY
  R119 Plasma Desktop

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

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


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread David Edmundson
davidedmundson added a comment.


  > Because you can miss or forgive that you activate it, we definitely NOT 
want to deactivate effect when you click desktop :)
  
  in this patch it DOES set active = false when you click on the desktop
  
  That's why I used the onDataChanged

REPOSITORY
  R114 Plasma Addons

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

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


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread Anthony Fieroni
anthonyfieroni added a comment.


  onVirtualDesktopChanged and onActivityChanged it looks good to deactivate 
effect with restoring windows state. Because you can miss or forgive that you 
activate it, we definitely NOT want to deactivate effect when you click desktop 
:) I think now it's looking good.

REPOSITORY
  R114 Plasma Addons

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

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


D13529: [minimizeAll] Do not try to restore window state on model change

2018-06-18 Thread David Edmundson
davidedmundson added inline comments.

INLINE COMMENTS

> main.qml:98
> +onActiveTaskChanged: {
> +root.active = false;
> +root.minimizedClients = [];

Oh!

The old c++ plugin had deactivate(bool) where the parameter indicated whether 
we restored the minimised windows or not. I had completely misread the old 
behaviour.

  onVirtualDesktopChanged: deactivate()
  onActivityChanged: deactivate()

should be doing what you're doing here not calling deactivate().

Can you change those, then  to that specific change.
(BUG: 395519)

---

as for onDataChanged vs onActiveTaskChanged

The reason I did this was if you click minimise all -> then click on the 
desktop we deactivate, something I don't think we want.

I don't fully understand what you're saying is wrong with the onDataChanged.

REPOSITORY
  R114 Plasma Addons

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

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


D13360: Touchpad KDED module: Convert to JSON metadata

2018-06-18 Thread Fabian Vogt
fvogt added a comment.


  This doesn't seem to be in Plasma/5.13.

REPOSITORY
  R119 Plasma Desktop

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

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