[Differential] [Commented On] D4204: Patch for plasmoid subsystem(containments/desktop) in plasma-desktop

[Konstantin Shtepa]

konstantinshtepa added a comment.


  In https://phabricator.kde.org/D4204#78701, @davidedmundson wrote:
  
  > That's quite a huge patch.
  >  Can you give a much bigger explanation on exactly what it's doing and why.
  >  "Fixes bug N" doesn't really explain what the original problem was.
  
  
  Sorry, diff not supposed to be send here today and in that kind of state. And I don't see here any way to delete this diff and close this theme. Because of this information is not fully ready.
  I would tomorrow add comments to code about what code does and why.

INLINE COMMENTS

> davidedmundson wrote in AppletAppearance.qml:61
> why do you do
> 
> property int foo
> Binding on foo {
> value: Math.max()
> }
> 
> instead of just
> 
> property int foo: Math.max
> 
> ?

Binding on properties used because these properties assigned in JS(in handlers onMinimumWidthChanged and others) as lvalue and it breaks normal bindings. If someone could bring better idea how to handle situation when minimum size > maximum size without these bindings then they can be removed.

> davidedmundson wrote in AppletAppearance.qml:226
> Point is a native QML type.
> http://doc.qt.io/qt-5/qml-point.html
> 
> property point lastPoint
> 
> then use that in your logic above.
> It can be better because you then get one signal when it changes not two.

You can go without links. I know Qt, commited there.
One signal when it doesn't have any handlers or bindings wouldn't change any weather in QML. Point too have it's negative sides because it needs inverpretation "lastPoint.x". The question here is maybe I should have used int too as a base to properties(to maintain policy of int-based sizes).

> davidedmundson wrote in AppletAppearance.qml:445
> if maximum is infinite, (so undefined in QtQuick.Layouts) set it to 1000000
> 
> why?
> It's far easier to test for isFinite() in other bits of code than a random big init

Just a few lines below this size is assigned for property with int as a base. Then this property is used in operations with others int-based kde sizes. And keep it as double is not a option because of default Qt maximumSize(Number.POSITIVE_INFINITY). So there goes 1000000. Is there better way?

REPOSITORY
  R119 Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: konstantinshtepa, #plasma
Cc: davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170119/20773946/attachment.html>