<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="https://git.reviewboard.kde.org/r/118406/">https://git.reviewboard.kde.org/r/118406/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 5th, 2014, 8:36 a.m. UTC, <b>Thomas Pfeiffer</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;">Usability review:
Since I lack the skills to picture it from the diff: When exactly is the notification shown? Is it shown as soon as the media is supposed to be played? If so, I think it could be done in a more subtle way: Grey out the entry in the playlist and just skip over it to the next entry int he playlist (like Amarok does) and show the message in a tooltip on mouseover on a pointer device or on tap on a touch device.
If it's already done in a similar way, then it's fine for me ;)</pre>
</blockquote>
<p>On June 5th, 2014, 8:50 a.m. UTC, <b>R.Harish Navnit</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've added a screenshot which should help. This is a little different from the way amarok deals with the same error. If the media is inaccessible, the message is shown in the player screen.
Is it okay, If I go ahead in the way I am or should look to implement something similar to what amarok does ?? </pre>
</blockquote>
<p>On June 5th, 2014, 9:29 a.m. UTC, <b>Thomas Pfeiffer</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;">Does PMC really have to be restarted to detect that the location is now accessible? That's not really a nice situation.
The whole message seems overly dramatic to me, especially in a playlist. Imagine you have a playlist with 50 songs in it, and three of them are on a location which isn't accessible. What would you prefer?
a) Having to close PMC, make the location accessible and then restart it
b) Just skip over those three songs and happily listen to the other 47
If we're not in a playlist, the problem should rarely occur because the browser only shows currently accessible media anyway, right?
</pre>
</blockquote>
<p>On June 5th, 2014, 9:52 a.m. UTC, <b>R.Harish Navnit</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;">So, like I'd mentioned in the mail(sorry about that).
1.PMC need not be restarted to get to detect that the location is accessible. I think the message might have been ambiguous. The user is expected to mount
all drives containing media and then run the application again.
2. You can skip over to the next media in a playlist. That's how I envisage the fix.
Please let me know things have to be done differently.</pre>
</blockquote>
<p>On June 5th, 2014, 11:03 a.m. UTC, <b>Thomas Pfeiffer</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;">The problem is that currently, the message is very scary. The skip controls are still active, but the user is presented with that very prominent error message telling her that she has to make sure that the file becomes accessible and restart the application. Plus, the playlist won't continue unless the user actively skips to the next entry, right?
The way Amarok does it is way less scary and disruptive since it silently skips. The only problem with Amarok's solution is that there is no way to find out why it skipped. That's why I suggested to offer a tooltip so users who wonder why a track was skipped can find out what the problem is. The message showing up in the tooltip could be like the one you've proposed, though I would change it to "Could not open the file [path]. To play this file, make sure it is accessible and restart Plasma Mediacenter"
</pre>
</blockquote>
<p>On June 5th, 2014, 1:47 p.m. UTC, <b>Shantanu Tushar</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;">Bah, this skipped me as well. There is absolutely no need to restart the app here, if the file is available, simply clicking play again should work.
Also, after Thomas pointed it out, in a playlist it actually makes sense to simply skip the missing media. However, if you play a single media (without the playlist) then this message seems appropriate.
@Thomas, does that sound right?
Finally, I'd suggest "and try again", insteasd of "and restart the application".</pre>
</blockquote>
<p>On June 7th, 2014, 11:14 a.m. UTC, <b>R.Harish Navnit</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 find another discrepancy here, simply play again doesn't work even if the file is available(after initially being inaccessible). There is no need to restart the app here, but the only way that I'm able to play the media is by playing either the next or previous media and then returning to the current media. Clicking play again doesn't seem to work for me. I don't find this too much of an issue btw, just reported though.
>From the discussions that we've had ^^, I'm inferring that it is expected that this fix works similar to the way in which amarok handles the same situation, and I shall proceed along the same lines. </pre>
</blockquote>
<p>On June 10th, 2014, 9:05 a.m. UTC, <b>Shantanu Tushar</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;">Well then its a bug, ideally clicking play again should work. This review should only go in once that bug is fixed imo (because it should change "please restart" to "please try again")
About the behavior, in my previous comment I have suggested that this error should only come when the played media is *not* in the current playlist. If it is in playlist, we should silently skip it.</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;">Okay, I'll look into that. I might not be able to work this week, but next week looks fine ;)</pre>
<br />
<p>- R.Harish </p>
<br />
<p>On June 5th, 2014, 8:47 a.m. UTC, R.Harish Navnit wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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, Shantanu Tushar and Sinny Kumari.</div>
<div>By R.Harish Navnit.</div>
<p style="color: grey;"><i>Updated June 5, 2014, 8:47 a.m.</i></p>
<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=333764">333764</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-mediacenter
</div>
<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;">If a media(in a playlist) is located in an inaccessible location, then the user is notified about the same. </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;">1. Load media to a playlist.
2. Unmount the device containing media.
3. Check if the user is notified of the location being inaccessible
--yes, the user is notified
4. Mount the device containing media and play a media from playlist.
-- The media plays properly.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>mediaelements/mediaplayer/MediaPlayer.qml <span style="color: grey">(98f1d2c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118406/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/06/05/3dc148a5-c4da-4d27-a713-e63922cbcef8__wihtout_i18n.png">wihtout_i18n.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>