<table><tr><td style="">aacid 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/D10859">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/D10859#inline-120888">View Inline</a><span style="color: #4b4d51; font-weight: bold;">simgunz</span> wrote in <span style="color: #4b4d51; font-weight: bold;">dlgannotationsbase.ui:55</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Yes.</p>
<p style="padding: 0; margin: 8px;">Both Author and Add had 'a' as accelerator. This was actually working. Pressing 'a' twice switch between the two.</p>
<p style="padding: 0; margin: 8px;">Nonetheless I changed it to make the accelerators unique.</p>
<p style="padding: 0; margin: 8px;">Initially I set 'u' as accelerator for Author but this was conflicting with 'Move Up' and the accelerator of author was automatically changed to 't' once running Okular. (not sure why this did not happen also for 'a').</p>
<p style="padding: 0; margin: 8px;">Then I noticed that not setting the accelerator let Qt set one automatically, so I decided to not set it. Mainly because I though that it make sense to set it only if it has a logical sense like 'u' for Move Up or 'd' for Move Down, but given that 't' is a random letter of author I decided to let it be automatic.</p>
<p style="padding: 0; margin: 8px;">Maybe it is better to set one explicitly?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's not Qt. It's kxmlgui that does the autoaccelerator magic. In principle it's always better to set it manually, this way translators know there's an accelrator and can try to set one that makes sense themselves and not leave it to "randomness" of the autoaccelerator magic.</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/D10859#inline-121116">View Inline</a><span style="color: #4b4d51; font-weight: bold;">annotationwidgets.cpp:460</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; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">tmplabel3</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">setBuddy</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"> </span><span class="n"><span class="bright">m_start</span>StyleCombo<span class="bright"></span></span><span class="bright"> </span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">tmp</span>label<span class="bright">4</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">setBuddy</span></span><span class="p">(</span> <span class="n">m_<span class="bright">end</span>StyleCombo</span> <span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">gridlay2</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">addWidget</span></span><span class="p">(</span> <span class="n">m_<span class="bright">start</span>StyleCombo<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">1</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">1</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">Qt</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">AlignLeft</span></span> <span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">gridlay2</span><span style="color: #aa2211">-></span><span class="n">addWidget</span><span class="p">(</span> <span class="n">m_endStyleCombo</span><span class="p">,</span> <span style="color: #601200">2</span><span class="p">,</span> <span style="color: #601200">1</span><span class="p">,</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">AlignLeft</span> <span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">tmplabel3</span><span style="color: #aa2211">-></span><span class="n">setToolTip</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Only for PDF documents"</span><span class="p">)</span> <span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">tmplabel4</span><span style="color: #aa2211">-></span><span class="n">setToolTip</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Only for PDF documents"</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="bright"></span><span class="n"><span class="bright">formlayout</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">addRow</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"> </span><span class="n"><span class="bright">i18n</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"> </span><span style="color: #766510"><span class="bright">"Line end:"</span></span><span class="bright"> </span><span class="p"><span class="bright">),</span></span><span class="bright"> </span><span class="n"><span class="bright">m_end</span>StyleCombo</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">formlayout</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="n">label<span class="bright">ForField</span></span><span class="p">(</span> <span class="n">m_<span class="bright">start</span>StyleCombo<span class="bright"></span></span><span class="bright"> </span><span class="p"><span class="bright">)</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">setToolTip</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"> </span><span class="n"><span class="bright">i18n</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">"Only for PDF documents"</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span> <span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">formlayout</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">labelForField</span></span><span class="p">(</span> <span class="n">m_<span class="bright">end</span>StyleCombo<span class="bright"></span></span><span class="bright"> </span><span class="p"><span class="bright">)</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">setToolTip</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"> </span><span class="n"><span class="bright">i18n</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">"Only for PDF documents"</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span> <span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">you're going to have conflicts, AFAICS this "only for PDF documents" is gone.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>BRANCH</strong><div><div>fix-annot-config-dialog</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10859">https://phabricator.kde.org/D10859</a></div></div><br /><div><strong>To: </strong>simgunz, Okular, VDG, ngraham<br /><strong>Cc: </strong>davidhurka, aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen<br /></div>