<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/111187/">http://git.reviewboard.kde.org/r/111187/</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 23rd, 2013, 3:45 p.m. UTC, <b>Sinny Kumari</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;">According to me, Playlist icon should toggle. If user have to click on playlist button to view playlist, user will expect that clicking once again will hide it especially in touch devices.
One more issue, related to https://bugs.kde.org/show_bug.cgi?id=318816 . Opening associated audio/video file in PMC, first homescreen appear then playlist and then video is visible. It looks little awkward.
Expected behaviour: Only video player should be visible</pre>
</blockquote>
<p>On June 23rd, 2013, 4:15 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;">There might be users who expect that the playlist button will toggle. However, they'll realize its not once they click it as there is no visual feedback of it being toggle. The reason it makes sense to use the back button is because now the playlist is a fully-fledged page rather than a toolbar-ish thing.
The homescreen and playlist are added to the stack so to achieve consistency when the user presses back (or escape) from the playing video. This is intended behavior.</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;">Well if toggle can be taken off from the playlist button icon, then I believe it becomes more standardized. Which would be helpful for user( from user perspective). Becuase then back button will help him/her to get back to previous state. I hope I am not wrong?</pre>
<br />
<p>- Sujith</p>
<br />
<p>On June 23rd, 2013, 2:26 p.m. UTC, Shantanu Tushar 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, Akshay Ratan, Fabian Riethmayer, Sinny Kumari, and Sujith Haridasan.</div>
<div>By Shantanu Tushar.</div>
<p style="color: grey;"><i>Updated June 23, 2013, 2:26 p.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;">Using a PageStack for navigation in PMC has lots of benefits, the most important one is that it makes navigation flows easier to handle. We can remove old hacks, cleanup code that used to manage navigation.
On the user's end, this brings more intuitive navigation from homescreen to browser, to playlist etc. Plus, keyboard navigation is much easier to do.
This patch improves the UI flows and keyboard navigation. There are still few things that don't have key bindings, so only report issues that are regressions.</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;">Tested in my branch pagestack-shantanu, works awesome.</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>browsingbackends/localfiles/localfilesabstractbackend.cpp <span style="color: grey">(4e2b336)</span></li>
<li>browsingbackends/localfiles/localfilesabstractmodel.cpp <span style="color: grey">(158725c)</span></li>
<li>browsingbackends/metadatabackends/metadatamusicbackend/metadatamusiccomponents/MediaBrowser.qml <span style="color: grey">(074d5a8)</span></li>
<li>browsingbackends/onlineservices/youtube/youtubebackend.desktop <span style="color: grey">(648ead0)</span></li>
<li>mediaelements/CMakeLists.txt <span style="color: grey">(4894d36)</span></li>
<li>mediaelements/imageviewer/ImageViewer.qml <span style="color: grey">(27b93f5)</span></li>
<li>mediaelements/imageviewer/PictureStrip.qml <span style="color: grey">(187d687)</span></li>
<li>mediaelements/imageviewer/PictureStripDelegate.qml <span style="color: grey">(0c995bf)</span></li>
<li>mediaelements/keyhandler/KeyHandler.qml <span style="color: grey">(0821c2c)</span></li>
<li>mediaelements/keyhandler/Logic.js <span style="color: grey">(b574221)</span></li>
<li>mediaelements/mediabrowser/MediaBrowser.qml <span style="color: grey">(04261aa)</span></li>
<li>mediaelements/mediabrowser/MediaItem.qml <span style="color: grey">(2d6fac5)</span></li>
<li>mediaelements/mediacontroller/MediaController.qml <span style="color: grey">(5d62a0f)</span></li>
<li>mediaelements/mediaplayer/MediaPlayer.qml <span style="color: grey">(e6df21d)</span></li>
<li>mediaelements/mediawelcome/BackendsListDelegate.qml <span style="color: grey">(7b9713a)</span></li>
<li>mediaelements/mediawelcome/MediaWelcome.qml <span style="color: grey">(6c4554e)</span></li>
<li>mediaelements/playlist/Playlist.qml <span style="color: grey">(dfa6c8d)</span></li>
<li>mediaelements/playlist/PlaylistDelegate.qml <span style="color: grey">(a2b4fae)</span></li>
<li>mediaelements/qmldir <span style="color: grey">(2d543c5)</span></li>
<li>mediaelements/runtimedata/RuntimeData.qml <span style="color: grey">(8524b47)</span></li>
<li>shells/newshell/mainwindow.h <span style="color: grey">(544618f)</span></li>
<li>shells/newshell/mainwindow.cpp <span style="color: grey">(113748e)</span></li>
<li>shells/newshell/package/contents/ui/mediacenter.qml <span style="color: grey">(d533b60)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111187/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>