Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-20 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/#review27796 --- This review has been submitted with commit 265696b694559e09cad

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-20 Thread Anmol Ahuja
> On Feb. 20, 2013, 3:21 a.m., Matěj Laitl wrote: > > Looks good to me, will merge in a few moments. Anmol, please document the > > new shortcuts on > > http://userbase.kde.org/Amarok/Manual/References/KeybindingReference/AmarokShortcuts > > and the new hidden config options on relevant page u

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-20 Thread Matěj Laitl
On 20. 2. 2013 Darth Codus wrote: > On Feb 20, 2013 3:21 AM, "Matěj Laitl" wrote: > >This is an automatically generated e-mail. To reply, visit: > > http://git.reviewboard.kde.org/r/108964/ > > > > Ship it! > > > > Looks good to me, will merge in a few moments. Anmol, please document the >

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-19 Thread Darth Codus
Thanks :) I'll get to the documentation in a short while. But aren't conflicting default shortcuts bad user experience? On Feb 20, 2013 3:21 AM, "Matěj Laitl" wrote: >This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108964/ > > Ship it! > > Looks

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-19 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/#review27752 --- Ship it! Looks good to me, will merge in a few moments. Anmol,

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-19 Thread Anmol Ahuja
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/ --- (Updated Feb. 19, 2013, 3:37 p.m.) Review request for Amarok. Changes --

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-18 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/#review27646 --- Technically correct patch, but please address the remarks below

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-16 Thread Matěj Laitl
> On Feb. 16, 2013, 10:26 a.m., Ralf Engels wrote: > > src/configdialog/dialogs/PlaybackConfig.ui, line 220 > > > > > > Could you extend the whatsThis text to indicate what keyboard > > modificators you need to g

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-16 Thread Ralf Engels
> On Feb. 16, 2013, 10:26 a.m., Ralf Engels wrote: > > src/configdialog/dialogs/PlaybackConfig.ui, line 220 > > > > > > Could you extend the whatsThis text to indicate what keyboard > > modificators you need to g

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-16 Thread Darth Codus
The short backward seek shortcut ( Control + Left) conflicts with the default Previous Browser shortcut. Should I change mine to Alt + L/ R? Mamarok said you don't use WhatsThis text in KDE anymore, so should the shortcut descriptions just go in the handbook? On Sat, Feb 16, 2013 at 5:57 PM, Da

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-16 Thread Darth Codus
So all I need to do is add a description of the shortcut keys to the whatsthis text? Does the following sound okay: The length of a short seek (Control + Left/ Right), in seconds. The length of a medium seek (Left/ Right), in seconds. The length of a long seek (Shift + Left/ Right), in seconds.

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-16 Thread Ralf Engels
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/#review27540 --- Technically OK. This whole seek options bloat up the code a lit

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-15 Thread Anmol Ahuja
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/ --- (Updated Feb. 15, 2013, 11:57 a.m.) Review request for Amarok. Changes -

Re: Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-15 Thread Matěj Laitl
> On Feb. 15, 2013, 10:13 a.m., Edward Hades Toroshchin wrote: > > Not judging if this is needed at all, I see the following problems: > > > > 1. This removes the "Left" and "Right" shortcuts, which users may have > > gotten used to. > > > > 2. Usually (in games and photo editing software, for

Review Request 108964: Added 3 different seek options (with customizable seek lengths)

2013-02-15 Thread Anmol Ahuja
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108964/ --- Review request for Amarok. Description --- Added 3 different seek op