<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/108480/">http://git.reviewboard.kde.org/r/108480/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 20th, 2013, 2:34 a.m. UTC, <b>Maarten De Meyer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This change is wrong imho. If 'Confirm logout' is disabled it should not ask for confirmation. Otherwise what's the point of that option? Showing the dialog depending on the button clicked breaks UI expectations a lot more than the presence of ellipses.
Perhaps remove the ellipses if the confirmation is disabled? (And show them in Kickoff/Tool box when enabled)
If people keep losing work due to hitting 'leave' by accident, there is a solution for that: Confirm logout.
Note: just my personal opinion and I'm not a plasma developer.</pre>
</blockquote>
<p>On January 21st, 2013, 9:34 a.m. UTC, <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i tend to agree with Marteen.
I think is not entirely nice that option is here at all since can have potentially dangerous results, but since is there it has to have a coherent behavior everywhere.
maybe what could be done is to remove the "..." in case confirmation is disabled</pre>
</blockquote>
<p>On January 21st, 2013, 12:22 p.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">+1 on removing the '...' when this setting is on. will drop this review as a result.
thanks for the submission, though, Jacob!</pre>
</blockquote>
<p>On January 21st, 2013, 11:32 p.m. UTC, <b>Jacob Welsh</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I can see the logic to this, however it raises some more issues. 1) The translations for "Leave..." would have to be changed, and the "..." appended programatically depending on the setting. That might not even make sense for all languages... what about right-to-left? If not, both versions would have to be translated. And same for the kickoff items. If this is the best solution though, I guess we'll just have to deal with it. 2) The "Leave" button is not quite the same as the kickoff options, because they explicitly specify a particular form of leaving. "Leave" is not clear whether it means shutdown, logout, sleep or what. This is why it makes sense to me to always prompt: how would you like to leave? Whereas with kickoff, the user has already provided that info, so the prompt is just a timeout to prevent accidents (which I'd prefer to disable in my own setup). Other solutions to #2 might be adding explicit shutdown/logout options to the desktop context menu (cluttering it), or having the text change based on the default leave option (complicating the code quite a bit).</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">for the translation, it would of course have to be two different strings, i18n("Leave...") and i18n("Leave"), the onle way it can be sure it works on any language</pre>
<br />
<p>- Marco</p>
<br />
<p>On January 19th, 2013, 7:37 a.m. UTC, Jacob Welsh wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Jacob Welsh.</div>
<p style="color: grey;"><i>Updated Jan. 19, 2013, 7:37 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Description from bug:
There's a logout option in the desktop context menu named "Leave...". The presence of ellipses indicates to the user that there will be a confirmation dialog. However, if you've disabled "Confirm logout" in System Settings / Startup and Shutdown / Session Management, no such confirmation will be given. This breaks UI expectations and could lead to loss of unsaved work. (I still prefer to disable confirmation for the case of the kickoff menu items, which don't have ellipses and are harder to hit "by accident".)</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">None. Sorry, I gave up on getting all the KDE components built from git for the time being; was getting one error after another. If there's a quick way to test just the plasma workspace I'd be happy to try.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=313480">313480</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/generic/containmentactions/contextmenu/menu.cpp <span style="color: grey">(bfd60a1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108480/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>