Review Request 124773: JJ Bug - 300979 : Add some shortcuts for various actions in createActions() in MainWindow.

[Myriam Schweingruber]

> On Sept. 13, 2015, 6:59 p.m., Olivier Churlaud wrote:
> > Regarding the code, everything seems fine. Now the real question is about the shortcuts. Are they really good? Logical?
> > 
> > I'm not the one to judge on this. But a Shift+E for instance might have bad repercussions (if you type Editors in the search field, what happens? Is the slot triggered?)
> > 
> > If Mamarok says it's ok, then ship it.
> 
> Aditya Dev Sharma wrote:
>     Well yes, there could be bad repurcussions for having same shortcuts. But that seems to be different for different people having different systems. For eg. I could use a shortcut already used above, for launching say yakuake. The user could still change these shortcuts from the GUI. So making absolutely unique shortcuts is difficult. I just took care that the shortcuts did not match any previous Global Shortcuts or even the ones used before.
>     
>     I'll wait for Mamarok :)
> 
> Myriam Schweingruber wrote:
>     Did you check the KDE Human Interface Guidelines here as well? -> https://techbase.kde.org/Projects/Usability/HIG/Keyboard_Shortcuts
>     
>     I am still a bit puzzled that you use Shift+Ctrl+E to export a playlist (when Ctrl+E triggers Track editing) and then Shift+E to show the Equalizer, doesn't seem that logical to me...
>     With logical I mean that the same shortcuts in variation should be useds to enable and disable the same things, so if the letter E is used for an Edit function, it should not be used for something totally different in another combination. If the shortcuts are not logical I am not sure those are really useful. A combination with S to show something might be a good path to explore.
>     
>     On the other hand our existing shortcuts don't seem to be very logical either *sigh*.
>     
>     About different people having different systems: we don't cater for other desktop systems, for us what is relevant is what is in use in KDE and Amarok. Currenlty GNOME for example uses different shortcuts already, so we really can't take this into consideration anyway. If our shortcuts conflict with other people's private selections, it is again not something we should care, as they can change their setting quite easily.
> 
> Olivier Churlaud wrote:
>     I'm wondering why we add this shortcuts? Every one has their own preference, as you said Myriam, and one can change it. So why don't we let it empty and the users choose directly what fits the best?

The idea was to make Amarok completely navigable by keyboard: https://bugs.kde.org/show_bug.cgi?id=300979 as a Junior Job for newcomers to get familiar with our code. I personally think this is not really useful to implement and a waste of ressources, but I didn't create those Junior Jobs


- Myriam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124773/#review85318
-----------------------------------------------------------


On Sept. 6, 2015, 7:30 p.m., Aditya Dev Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124773/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2015, 7:30 p.m.)
> 
> 
> Review request for Amarok, Myriam Schweingruber and Bart Cerneels.
> 
> 
> Bugs: 300979
>     https://bugs.kde.org/show_bug.cgi?id=300979
> 
> 
> Repository: amarok
> 
> 
> Description
> -------
> 
> JJ : Bug - 300979 : Patch to add some shortcuts for various actions in createActions() in MainWindow
> 
> 
> Diffs
> -----
> 
>   src/MainWindow.cpp 22fb8ce 
> 
> Diff: https://git.reviewboard.kde.org/r/124773/diff/
> 
> 
> Testing
> -------
> 
> Built and tested.
> Removed the shortcuts that matched either the Amarok Global Shortcuts or the Standard Shortcuts of KDE.
> 
> Clear Playlist : CTRL + SHIFT + C  ---------------> SLOT(clear() )
> Add Stream : CTRL + SHIFT + J      ---------------> SLOT(slotAddStream() ) 
> Export Playlits : CTRL + SHIFT + E ---------------> SLOT(exportPlaylist() )
> Bookmarks Manager : CTRL + I       ---------------> SLOT(slotShowBookmarkManager() )
> Equalizer : SHIFT + E              ---------------> SLOT(slotShowEqualizer() )
> Cover Manager : CTRL + ALT + C     ---------------> SLOT(slotShowCoverManager() )
> Synchronize Statistics : SHIFT + S ---------------> SLOT(synchronize() )
> 
> Bookmark Media Sources View : SHIFT + B ----------> SLOT(bookmarkCurrentBrowserView() )
> Bookmark Playlist Setup : ALT + B  ---------------> bookmarkCurrentPlaylistView() )
> Bookmark Context Applets : CTRL + SHIFT + B ------> bookmarkCurrentContextView() ) 
> Disable Dynamic Playlist : ALT + SHIFT + D -------> connected inside dynamic playlist category
> 
> 
> About Amarok : ALT + F10  -----------------> SLOT(showAbout())
> Diagnostics  : ALT + F8    -----------------> SLOT(slotShowDiagnosticsDialog())
> Report bug   : ALT + F9   -----------------> SLOT(showReportBug())
> 
> 
> Thanks,
> 
> Aditya Dev Sharma
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20150914/e0ae2ebc/attachment-0001.html>