<table><tr><td style="">camiloh marked 2 inline comments as done.<br />camiloh 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/D22974">View Revision</a></tr></table><br /><div><div><p>I have worked on the duplicating icons, by not using the contentItem to draw the icons and label, and instead use the style implementation. The only thing missing would be the dropdown icon for actions with submenus, I'm anchoring that icon to the right con the contentItem. I will update this patch.<br />
I will push my proposal for duplicating icons, it can be reviewed. And then I can split this patch into different parts as suggested by Aleix.</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/D22974#inline-129607">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Action.qml:76</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Can you check that we're not missing some API? is ActionIconGroup a subset of what Qt offers?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">from Kirigami actionIconGroup:<br />
string name<br />
string source<br />
int width<br />
int height<br />
color color</p>

<p style="padding: 0; margin: 8px;">from QQC2 icon property:<br />
icon.name<br />
icon.source<br />
icon.width<br />
icon.height<br />
icon.color</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/D22974#inline-129605">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ActionToolBar.qml:184</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">If <tt style="background: #ebebeb; font-size: 13px;">ourAction</tt> doesn't have a visible property, won't <tt style="background: #ebebeb; font-size: 13px;">ourAction.visible</tt> be undefined?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think it then gets marked as visible = true</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/D22974#inline-129608">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">PrivateActionToolButton.qml:89</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Wouldn't it be control.action.icon.name?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">yes. fixed now on new commit</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R169 Kirigami</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22974">https://phabricator.kde.org/D22974</a></div></div><br /><div><strong>To: </strong>camiloh, Kirigami, mart<br /><strong>Cc: </strong>astippich, apol, plasma-devel, fbampaloukas, domson, dkardarakos, davidedmundson, mart, hein<br /></div>