<table><tr><td style="">gjditchfield 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/D29698">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/D29698#inline-170089">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mlaurent</span> wrote in <span style="color: #4b4d51; font-weight: bold;">koeventpopupmenutest.cpp:222</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Indeed it's define here but I think it's not real useful to test text directly.<br />
Testing objectName is more logical as previously.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Perhaps the entire defaultMenuEventVisible() function should be deleted?</p>

<p style="padding: 0; margin: 8px;">The old code checked that the "create note" and "create todo" items were visible, but createTodoFromEvent() and createNoteFromEvent() have already tested that.  The old code also tested that the "create event" item exists but is <strong>not</strong> visible -- that is an implementation detail of KOEventPopupMenu, and seems like a poor test to me.</p>

<p style="padding: 0; margin: 8px;">I added tests for the existence of the current default menu items, because I thought it was in the spirit of the old code, but my new tests don't check that the items actually <strong>do</strong> anything, so they are nearly pointless.</p>

<p style="padding: 0; margin: 8px;">I'd like to delete both defaultMenuTodoVisible() and defaultMenuEventVisible().</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R210 KOrganizer</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D29698">https://phabricator.kde.org/D29698</a></div></div><br /><div><strong>To: </strong>gjditchfield, mlaurent<br /><strong>Cc: </strong>mlaurent, dvratil, kde-pim, fbampaloukas, dvasin, rodsevich, winterz, vkrause, knauss<br /></div>