<table><tr><td style="">kossebau marked 4 inline comments as done.<br />kossebau 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/D9751" 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/D9751#inline-44290" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">configWeatherStation.qml:101</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Can you verify that toggling the MenuItem does not break this binding? It shouldn't cause much trouble, though, as you only change selected services in response to this being toggled, right?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Good catch.<br />
Yes, it breaks it, also creates a binding loop which QML warns about. Somehow completely missed that. Shows why I can not put QML expert on my business card :)</p>

<p style="padding: 0; margin: 8px;">Tried to solve by removing the bindings and instead adding a readonly proxy(?) property for the model item to the menu item and then updating the values of the menu item and the model item in the respective changed/toggled handlers. Seems to work, is that a proper solution? Had not found anything to copycat on a quick search, so assembled this from implementation hints across the places. How expensive is such a proxy property?</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/D9751#inline-44289" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">configWeatherStation.qml:147</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is that the actual icon we use for such popups? A blue flag isn't very descriptive imho, though I can see that Dolphin's "Service" menu also uses that. (I can't think of a better solution, though, other than the funnel filter icon)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Agreed that the flag is quite strange.<br />
While the icon name might be good enough, the current Breeze icon theme implementation with the flag is rather strange and get a new approach. No idea which global(?) metapher is referred to with a flag for services...</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/D9751#inline-44288" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">servicelistmodel.h:54</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">There's a <tt style="background: #ebebeb; font-size: 13px;">Qt::CheckStateRole</tt>, using that might save you some boilerplate code in various places</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Thanks, missed that. Sadly no rolename set, so I still have to do this myself. But yes, saved some lines still :P</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R114 Plasma Addons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9751" rel="noreferrer">https://phabricator.kde.org/D9751</a></div></div><br /><div><strong>To: </strong>kossebau, Plasma<br /><strong>Cc: </strong>broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>