[rekonq] Re: Review Request: new: calling favorite pages via shortcut

[Pierre Rossi]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101043/#review2489
-----------------------------------------------------------


Welcome to Rekonq ! :)
Apart from the naming, I think the index logic could get a bit more elegant and robust.


src/mainview.h
<http://git.reviewboard.kde.org/r/101043/#comment2159>

    the name is a bit confusing, technically it is loading the favorite, not switching to it like you switch to a tab.



src/mainview.cpp
<http://git.reviewboard.kde.org/r/101043/#comment2158>

    you could make this slot take the index as a parameter and use QSignalMapper to establish the connections.
    
    http://doc.qt.nokia.com/latest/qsignalmapper.html



src/mainwindow.cpp
<http://git.reviewboard.kde.org/r/101043/#comment2160>

    You could use a signal mapper and do the mapping here. and the appropriate connections (actions to mapper within the loop, mapper to slot once you're out)


- Pierre


On April 7, 2011, 5:19 p.m., Thomas Murach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101043/
> -----------------------------------------------------------
> 
> (Updated April 7, 2011, 5:19 p.m.)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> This patch adds the following functionality: Pressing Ctrl + 1 up to Ctrl + 9 will load the favorite page with number 1 ... 9 in the current tab. I find this very useful when using Opera.
> This is my first patch. So please tell me if there is something to improve in my kind of coding / formatting / ...
> The code itself is pretty much copied from the function SwitchToTab() in MainView. 
> Numbers higher than 9 do not make sense as shortcuts (in my opinion) and are therefore omitted, as well as zero
> 
> 
> Diffs
> -----
> 
>   src/mainview.h acc2d8c 
>   src/mainview.cpp b34acc3 
>   src/mainwindow.cpp 7b4e3ee 
> 
> Diff: http://git.reviewboard.kde.org/r/101043/diff
> 
> 
> Testing
> -------
> 
> Yes. Couldn't find any problems.
> For example pressing Ctrl + _Number_X_ doesn't crash if you have less than _Number_X_ favorite pages
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110407/4e7bc060/attachment-0001.htm