Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.
On June 5, 2014, 8:36 a.m., Thomas Pfeiffer wrote: 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 ;) R.Harish Navnit wrote: 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 ?? Thomas Pfeiffer wrote: 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? R.Harish Navnit wrote: 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. Thomas Pfeiffer wrote: 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 Shantanu Tushar wrote: 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. 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. - R.Harish --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/#review59281 --- On June 5, 2014, 8:47 a.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/ --- (Updated June 5, 2014, 8:47 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 333764 http://bugs.kde.org/show_bug.cgi?id=333764 Repository: plasma-mediacenter Description --- If a media(in a playlist) is located in an inaccessible location, then the user is
Re: Review Request 118581: Consider Super_L and Super_R as modifiers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118581/#review59507 --- Ship it! Ship It! - David Faure On June 6, 2014, 12:39 a.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118581/ --- (Updated June 6, 2014, 12:39 a.m.) Review request for KDE Frameworks, Plasma and Vishesh Handa. Bugs: 335316 https://bugs.kde.org/show_bug.cgi?id=335316 Repository: kxmlgui Description --- Consider Super_L and Super_R as modifiers Without this patch, I can't use the meta key to assign shortcuts, as Super_L and Super_R are not considered as modifiers, so when I press meta (Super_L on my system), the shortcut is immediately accepted, before I get the chance to press another key. This patch requires the fix in https://bugreports.qt-project.org/browse/QTBUG-38428 to be applied. With both patches, KKeySequenceWidget works for me. BUG:335316 Diffs - src/kkeysequencewidget.cpp b6fcd207a1d18466f4a747e1a0b4b58107c82871 Diff: https://git.reviewboard.kde.org/r/118581/diff/ Testing --- Tried to assign meta + something in global shortcuts KCM, fails without patch (see screenshot in the linked bugreport), works correctly with patch. Thanks, Sebastian Kügler ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Locale Name Primer
Locale Names.
A quick primer on Locale Names, seeing as we've had a few issues in
the last couple of days. I can't claim perfect knowledge, so feel free
to point out where I am wrong :-)
TL;DR:
* Don't use QLocale::bcp47Name().
* Use QLocale::name(), but may need to modify the results.
* You usually want QLocale().name() and not QLocale::system().name()
* Don't assume language code is alpha-2, it may be alpha-3
* Always use case insensitive comparisons.
* I'll improve support in Qt 5.4.
POSIX Locale:
* What we use on Linux systems and in glibc.
* Format is lang_REGION.encoding@variant.
* How many of those componants are included can depend on the system
or implementation. In most cases the language and region are always
included, with the encoding and variant optional.
* lang is ISO 639 alpha-2 or alpha-3 (so QLocale().name().left(2) is
not valid!) , usually in lower case
* REGION is ISO 3166 alpha-2, usually in upper case
* Many distros explicitly add .utf8 or .UTF-8 for Unicode, e.g. on
openSUSE en_GB.utf8 uses UTF-8 but en_GB uses ISO-8859-1.
* The variant is usually used to change the script, but doesn't use an
ISO code for this e.g. sr_RS uses Cyrillic script but sr_RS@latin
uses Latin script
* The variant can change other options like the currency, e.g. de_DE@euro
* Always use case-insensitive comparison as case of codes is meaningless
* Run locale -a to see what your distro has installed
BCP47 Locale:
* IETF RFC, used in Unicode, W3C standards, etc.
* Used in Windows Vista and later, where it replaces the old LCID.
* Basic format is lang-Script-REGION-variants-extensions
* Always uses hyphen as a subtag separator
* Always uses minimum subtags required to uniquly identify locale,
e.g. de-Latn-DE will be reduced to de as Latn and DE are the
assumed defaults.
* lang is ISO 639 alpha-2 or alpha-3 code, usually in lower case
* Script is ISO 15924 alpha4 code usually in title case
* REGION is ISO 3166 alpha 2 code, usually in upper case
* variant are registered variant codes
* extensions are registered extension codes
* Always use case-insensitive comparison as case of codes is meaningless
Qt 5.3 support:
* name() always returns lang_REGION, except where AnyCountry is set
or C, never returns encoding or variant
* bcp57Name() returns the minimal BCP47 name
* No direct may to get lang or country code, need to use
QLocale().name().split('_').at(x)
Needed in Qt 5.4:
* languageCode()
* countryCode()
* scriptCode()
* posixName()
* encoding?
Cheers!
John.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118581: Consider Super_L and Super_R as modifiers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118581/#review59532 --- Ship it! Looks good to me. Can you check if we need to update the the similar code in in kdeclarative-kquickcontrols too. (or poke me repeatedly to do it). - David Edmundson On June 6, 2014, 12:39 a.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118581/ --- (Updated June 6, 2014, 12:39 a.m.) Review request for KDE Frameworks, Plasma and Vishesh Handa. Bugs: 335316 https://bugs.kde.org/show_bug.cgi?id=335316 Repository: kxmlgui Description --- Consider Super_L and Super_R as modifiers Without this patch, I can't use the meta key to assign shortcuts, as Super_L and Super_R are not considered as modifiers, so when I press meta (Super_L on my system), the shortcut is immediately accepted, before I get the chance to press another key. This patch requires the fix in https://bugreports.qt-project.org/browse/QTBUG-38428 to be applied. With both patches, KKeySequenceWidget works for me. BUG:335316 Diffs - src/kkeysequencewidget.cpp b6fcd207a1d18466f4a747e1a0b4b58107c82871 Diff: https://git.reviewboard.kde.org/r/118581/diff/ Testing --- Tried to assign meta + something in global shortcuts KCM, fails without patch (see screenshot in the linked bugreport), works correctly with patch. Thanks, Sebastian Kügler ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118548: Port libtaskmanager away from QDesktopWidget
On Fri, Jun 6, 2014 at 10:20 PM, Luca Beltrame lbeltr...@kde.org wrote: In data venerdì 06 giugno 2014 17:22:52, Aleix Pol ha scritto: Can you give it another try? Now works perfectly. Thanks! -- Luca Beltrame - KDE Forums team KDE Science supporter GPG key ID: 6E1A4E79 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel Thanks to you, for caring, for testing and for putting up with my own mental chaos. :D Aleix ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 118613: Prevent crash when requesting icon with negative size
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118613/ --- Review request for Plasma. Bugs: 335939 http://bugs.kde.org/show_bug.cgi?id=335939 Repository: plasma-framework Description --- Prevent crash when requesting icon with negative size. Diffs - src/declarativeimports/core/iconitem.cpp 9e0cb36 Diff: https://git.reviewboard.kde.org/r/118613/diff/ Testing --- works Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118613: Prevent crash when requesting icon with negative size
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118613/ --- (Updated June 8, 2014, 10:15 a.m.) Review request for Plasma. Bugs: 335939 http://bugs.kde.org/show_bug.cgi?id=335939 Repository: plasma-framework Description --- Prevent crash when requesting icon with negative size. Diffs - src/declarativeimports/core/iconitem.cpp 9e0cb36 Diff: https://git.reviewboard.kde.org/r/118613/diff/ Testing (updated) --- tried to resize some icons.. no longer crashes Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
