[rekonq] Re: Review Request: Changes to the context menu
Benjamin Poulain
ikipou at gmail.com
Thu Jan 13 00:47:53 CET 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100374/#review862
-----------------------------------------------------------
src/findbar.cpp
<http://git.reviewboard.kde.org/r/100374/#comment650>
Instead, I would just add
FindBar::setText(const QString&).
The caller would do:
findBar->setText(selectedText())
findBar->show().
What you did here is duplicate some of the logic of setVisible() for this special case.
src/mainwindow.cpp
<http://git.reviewboard.kde.org/r/100374/#comment656>
That is a bit hackish, see other comment.
src/webview.cpp
<http://git.reviewboard.kde.org/r/100374/#comment655>
I think this is a bit hackish. This will go about reusability of the target slot.
I would personnaly do a new slot in WebView:
void findSelectedTextInPage() and connect it to the action.
src/webview.cpp
<http://git.reviewboard.kde.org/r/100374/#comment654>
Coding style:
if (ReKonfig::showDeveloperTools())
src/webview.cpp
<http://git.reviewboard.kde.org/r/100374/#comment653>
Coding style:
if (ReKonfig::showDeveloperTools())
src/webview.cpp
<http://git.reviewboard.kde.org/r/100374/#comment652>
Coding style:
if (!ReKonfig::alwaysShowTabBar() && mainwindow->mainView()->count() == 1)
src/webview.cpp
<http://git.reviewboard.kde.org/r/100374/#comment651>
Coding style:
if (ReKonfig::showDeveloperTools())
Do not hesitate to break the patch in two :
1) remove the superflous entries in the menu
2) hide the developer tools depending on the settings
They are indenpant changes.
- Benjamin
On Jan. 12, 2011, 11:35 p.m., Felix Rohrbach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100374/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2011, 11:35 p.m.)
>
>
> Review request for rekonq.
>
>
> Summary
> -------
>
> This patch contains the changes to the context menu which we decided in our meeting:
>
> 1. Hide new tab entry if it is not necessary.
> 2. Print and Search action: I removed the print action. The search action is now in the "search with" submenu if some text is selected.
> 3. Hide the development menu entries and add an option to show them.
>
> Clone: git://anongit.kde.org/clones/rekonq/felixr/rekonq
> Branch: meetingMenuChanges
>
>
> Diffs
> -----
>
> src/findbar.h ed85676
> src/findbar.cpp 158849f
> src/mainwindow.h 7dfc186
> src/mainwindow.cpp 599295f
> src/rekonq.kcfg cda76d8
> src/settings/settings_webkit.ui 58fbe45
> src/webview.cpp 49fa103
>
> Diff: http://git.reviewboard.kde.org/r/100374/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Felix
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110112/0df0beb7/attachment-0001.htm
More information about the rekonq
mailing list