<table><tr><td style="">hein added inline comments.
</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/D8916" rel="noreferrer">View Revision</a></tr></table><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/D8916#inline-39802" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fonts.cpp:570</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">*</span></span><span class="bright"></span><span class="n"><span class="bright">it</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">write</span>Font</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">emit</span></span><span class="bright"> </span><span style="color: #004012"><span class="bright">small</span>Font<span class="bright">Changed</span></span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">setNeedsSave</span><span class="p">(</span><span style="color: #304a96">true</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Instead of always setting this to true in a prop setter, you need to implement a method that checks whether the new value is actually different from the stored configuration and then sets true or false based on that. Otherwise you are turning on the 'Apply' button but never turning it off again. Cf. LaunchFeedback::updateNeedsSave() in <a href="https://phabricator.kde.org/D8911" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D8911</a>.</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/D8916#inline-39803" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">main.qml:175</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">enabled:</span> <span style="color: #004012">dpiCheckBox</span><span class="p">.</span><span style="color: #004012">checked</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">value:</span> <span style="color: #004012">kcm</span><span class="p">.</span><span style="color: #004012">fontAASettings</span><span class="p">.</span><span style="color: #004012">dpi</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">onValueChanged:</span> <span style="color: #004012">kcm</span><span class="p">.</span><span style="color: #004012">fontAASettings</span><span class="p">.</span><span style="color: #004012">dpi</span> <span style="color: #aa2211">=</span> <span style="color: #004012">dpiSpinBox</span><span class="p">.</span><span style="color: #004012">value</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This and similar bindings will break when the user changes the spinbox value, so after a manual adjustment e.g. defaults() won't work any longer. It needs a seperate Connections à la <a href="https://phabricator.kde.org/D8911" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D8911</a>.</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/D8916" rel="noreferrer">https://phabricator.kde.org/D8916</a></div></div><br /><div><strong>To: </strong>mart, Plasma, VDG<br /><strong>Cc: </strong>hein, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>