<table><tr><td style="">hein added a subscriber: mart.<br />hein 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/D21829">View Revision</a></tr></table><br /><div><div><p>I'm in principle quite OK with this minus that other patch I need to be written first to move DragHelper out into the lib (see other comment). Pitch: If you do that, it'll make it much easier to make the other menus (and other things) act similarly later, with smaller patches there.</p>
<p>However we need a collective decision that this way to handle tap-and-hold is the one we want to go forward with - this patch basically makes a call to resolve <a href="https://phabricator.kde.org/T10783" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">T10783</a> in a particular way - and needs some more buy-in e.g. from <a href="https://phabricator.kde.org/p/mart/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@mart</a> for that reason.</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/D21829#inline-123721">View Inline</a><span style="color: #4b4d51; font-weight: bold;">draghelper.cpp:91</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; "> <span class="n">drag</span><span style="color: #aa2211">-></span><span class="n">setPixmap</span><span class="p">(</span><span class="n">icon</span><span class="p">.</span><span class="n">pixmap</span><span class="p">(</span><span class="n">m_dragIconSize</span><span class="p">,</span> <span class="n">m_dragIconSize</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">touch</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">drag</span><span style="color: #aa2211">-></span><span class="n">setHotSpot</span><span class="p">(</span><span class="n">QPoint</span><span class="p">(</span><span class="n">m_dragIconSize</span><span style="color: #aa2211">*</span><span style="color: #601200">0.5</span><span class="p">,</span><span class="n">m_dragIconSize</span><span style="color: #aa2211">*</span><span style="color: #601200">0.95</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm not happy with hardcoding these values. If we decide to do touch drag this way, we're going to see this being duplicated all around the codebase - with subtly different values, etc. If we want it that way we should centralize it somewhere.</p>
<p style="padding: 0; margin: 8px;">We probably need to put DragHelper to plasma-framework - there's similar classes in FV and TM too we can consolidate. This isn't that much work and I'll gladly help mentoring the patch.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>BRANCH</strong><div><div>kickoff</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21829">https://phabricator.kde.org/D21829</a></div></div><br /><div><strong>To: </strong>steffenh, Plasma, hein, ngraham<br /><strong>Cc: </strong>mart, sbergeron, davidedmundson, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol<br /></div>