Code review comment for lp:~meese/slingshot/fix-1084886

Revision history for this message
David Gomes (davidgomes) wrote :

You shouldn't be checking if event.button is 3. I believe there is a
cleaner approach but I'm on my phone now. Will look for it later.
On Sep 3, 2014 10:40 AM, "meese" <email address hidden> wrote:

> meese has proposed merging lp:~madelynn-r-may/slingshot/fix-1084886 into
> lp:slingshot.
>
> Commit message:
> makes right click on search entry pastes clipboard contents
>
> Requested reviews:
> elementary Pantheon team (elementary-pantheon)
> Related bugs:
> Bug #1084886 in Slingshot: "No right click menu in search [$15]"
> https://bugs.launchpad.net/slingshot/+bug/1084886
>
> For more details, see:
>
> https://code.launchpad.net/~madelynn-r-may/slingshot/fix-1084886/+merge/233187
>
> Partially fixes 1084886 by making right click on the top search entry
> paste clipboard contents. Copy isn't really necessary for a search entry as
> a user is unlikely to want to copy information they just put in themselves.
> Also popup menus do not play nicely with slingshots window in its current
> form.
> --
>
> https://code.launchpad.net/~madelynn-r-may/slingshot/fix-1084886/+merge/233187
> Your team elementary Pantheon team is requested to review the proposed
> merge of lp:~madelynn-r-may/slingshot/fix-1084886 into lp:slingshot.
>
> === modified file 'src/SlingshotView.vala'
> --- src/SlingshotView.vala 2014-08-31 21:38:07 +0000
> +++ src/SlingshotView.vala 2014-09-03 09:39:35 +0000
> @@ -164,7 +164,15 @@
> dummy_search_entry = new Gtk.SearchEntry ();
> dummy_search_entry.placeholder_text = _("Search Apps…");
> dummy_search_entry.width_request = 250;
> - dummy_search_entry.button_press_event.connect ((e) => {return
> e.button == 3;});
> + dummy_search_entry.button_press_event.connect ((event) => {
> + if (event.button == 3) {
> + var cb = Gtk.Clipboard.@get
> (Gdk.SELECTION_CLIPBOARD);
> + var text = cb.wait_for_text ();
> + dummy_search_entry.text = text;
> + return true;
> + }
> + return false;
> + });
>
> if (Slingshot.settings.show_category_filter) {
> top.attach (view_selector, 0, 0, 1, 1);
>
>
>

« Back to merge proposal