<table><tr><td style="">davidhurka 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/D21971">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D21971#559485" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D21971#559485</a>, <a href="https://phabricator.kde.org/p/simgunz/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@simgunz</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>To sum it up, if I understand correctly:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">we can remove <tt style="background: #ebebeb; font-size: 13px;">InstantPopup</tt> given that we can use <tt style="background: #ebebeb; font-size: 13px;">KActionMenu</tt> or <tt style="background: #ebebeb; font-size: 13px;">KSelectAction</tt> to provide that use case (so no need to raise exceptions)</li>
</ul></div>
</blockquote>

<p>Yes, but how? It can’t simply be forbidden in the constructor argument list. Remove it from the body of the constructor, so KActionMenu is set to MenuButtonPopup instead?</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">you agree in removing <tt style="background: #ebebeb; font-size: 13px;">DefaultLogic</tt></li>
</ul></blockquote>

<p>No, it is needed when other actions than the exclusive group are added to the menu.<br />
Example is <a href="https://phabricator.kde.org/D21195" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D21195</a>, where an action to configure the color modes is added to the <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Color Mode</span></span></span> menu. When you click that action in ImplicitDefaultAction mode, it would become the default action, and the toolbar tells you that you are in color mode “Configure...”.<br />
Other option is to allow only checkable actions to become the default action. But then, you can’t add real “actions” (opposed to “tools”). Imagine the user has some construction plan, and the application has several actions of kind “Mark for <job>”, and the user wants to apply one of these action to several items. Then it could be useful to collect all “Mark for <job>” actions in a ToggleActionMenu ImplicitDefaultAction mode, while the actions can not be checked.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>The change to <tt style="background: #ebebeb; font-size: 13px;">ToggleActionMenu::defaultAction</tt> is required, otherwise if <tt style="background: #ebebeb; font-size: 13px;">setDefaultAction</tt> is called on action1 manually and then action2 is checked, <tt style="background: #ebebeb; font-size: 13px;">defaultAction</tt> returns action1.</p></blockquote>

<p>I can’t follow you here. Yes, when in DefaultLogic mode, and setDefaultAction(action1) was called, defaultAction() returns action1. Maybe your thinking was focused on checkable actions so far?</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>[...]<br />
 Thinking forward, <tt style="background: #ebebeb; font-size: 13px;">ToggleActionMenu</tt> could become just a 'mode' of <tt style="background: #ebebeb; font-size: 13px;">KActionMenu</tt>. Right?</p></blockquote>

<p>Yes. But I would like <a href="https://bugs.kde.org/show_bug.cgi?id=413827" class="remarkup-link" target="_blank" rel="noreferrer">Bug 413827</a> to be resolved (so PopupMode enumerator is used).</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21971">https://phabricator.kde.org/D21971</a></div></div><br /><div><strong>To: </strong>davidhurka<br /><strong>Cc: </strong>ngraham, simgunz, okular-devel, johnzh, andisa, siddharthmanthan, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen, aacid<br /></div>