<table><tr><td style="">zzag 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/D23102">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/D23102#inline-130421">View Inline</a><span style="color: #4b4d51; font-weight: bold;">gladhorn</span> wrote in <span style="color: #4b4d51; font-weight: bold;">useractions.cpp:154-162</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I'm a bit unsure what you mean - the entire popup position trickery? I was wondering how much it's needed, but didn't want to break existing things. For now I assume the code is there for a reason - it seems to me that it want to move the popup high enough to not be positioned at the bottom in a way that it would be cut off. I'm not sure what Qt actually does... in theory I think it should move it up.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't see why we have to ensure that QMenu doesn't fall off screen. QMenu already does that for us. Besides that our code is incomplete(only one out of three cases is handled) and incorrect(<tt style="background: #ebebeb; font-size: 13px;">y + popupHeight >= area.height()</tt> is wrong).</p>
<p style="padding: 0; margin: 8px;">Remove <tt style="background: #ebebeb; font-size: 13px;">if (y != pos.top())</tt> and everything that goes inside.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D23102">https://phabricator.kde.org/D23102</a></div></div><br /><div><strong>To: </strong>gladhorn, KWin, romangg, zzag<br /><strong>Cc: </strong>zzag, romangg, kwin, LeGast00n, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart<br /></div>