<table><tr><td style="">konstantinshtepa added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4204" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D4204#78701" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D4204#78701</a>, <a href="https://phabricator.kde.org/p/davidedmundson/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@davidedmundson</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>That's quite a huge patch.<br />
Can you give a much bigger explanation on exactly what it's doing and why.<br />
"Fixes bug N" doesn't really explain what the original problem was.</p></div>
</blockquote>
<p>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.<br />
I would tomorrow add comments to code about what code does and why.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4204#inline-16661" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">AppletAppearance.qml:61</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">why do you do</p>
<p style="padding: 0; margin: 8px;">property int foo<br />
Binding on foo {<br />
value: Math.max()<br />
}</p>
<p style="padding: 0; margin: 8px;">instead of just</p>
<p style="padding: 0; margin: 8px;">property int foo: Math.max</p>
<p style="padding: 0; margin: 8px;">?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4204#inline-16662" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">AppletAppearance.qml:226</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Point is a native QML type.<br />
<a href="http://doc.qt.io/qt-5/qml-point.html" class="remarkup-link" target="_blank" rel="noreferrer">http://doc.qt.io/qt-5/qml-point.html</a></p>
<p style="padding: 0; margin: 8px;">property point lastPoint</p>
<p style="padding: 0; margin: 8px;">then use that in your logic above.<br />
It can be better because you then get one signal when it changes not two.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You can go without links. I know Qt, commited there.<br />
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).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4204#inline-16663" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">AppletAppearance.qml:445</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">if maximum is infinite, (so undefined in QtQuick.Layouts) set it to 1000000</p>
<p style="padding: 0; margin: 8px;">why?<br />
It's far easier to test for isFinite() in other bits of code than a random big init</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4204" rel="noreferrer">https://phabricator.kde.org/D4204</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>konstantinshtepa, Plasma<br /><strong>Cc: </strong>davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>