[Webkit-unassigned] [Bug 117631] [GTK] Provide search functionality to MiniBrowser

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117631





--- Comment #18 from Andres Gomez Garcia <agomez at igalia.com>  2013-07-23 04:45:23 PST ---
(In reply to comment #14)
> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:45
> > +    if (!g_strcmp0(text, ""))
> > +        return;
> 
> I guess we should finish the search operation in this case.

Done.

> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:77
> > +    case GTK_ENTRY_ICON_PRIMARY:
> > +        searchNext(searchBar);
> > +        break;
> 
> This is very weird, what I expect of a primary button like this one is a menu with search options, not just another button to jump to the next search result. So, I would simply remove it, it duplicates the next button functionality and makes the entry more cluttered.

As I've redesigned the whole thing, now it pops up a context menu.

> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:82
> > +        gtk_entry_set_text(GTK_ENTRY(searchBar->entry), "");
> > +
> > +        WebKitFindController *controller = webkit_web_view_get_find_controller(searchBar->webView);
> > +        webkit_find_controller_search_finish(controller);
> 
> Do we really need this? I think setting the entry text should be enough, since the changed signal will be emitted

Done.


> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:123
> > +    GtkWidget *closeButton = gtk_button_new_from_stock(GTK_STOCK_CLOSE);
> > +    gtk_box_pack_start(GTK_BOX(hBox), closeButton, FALSE, FALSE, 0);
> 
> The find bar is open, but also closed with the button in the toolbar, I would not duplicate the functionality. I agree it's confusing because the button in the toolbar works as a toggle button but it's not a toggle button. So I would either convert the tool button into a toggle button and remove this one or leave this one (using only the icon and moving it to the right of the find bar) and make the tool button do nothing when the find bar is already shown.

Done.

> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:138
> > +    GtkWidget *prevButton = gtk_button_new_from_stock(GTK_STOCK_GO_BACK);
> > +    gtk_box_pack_start(GTK_BOX(hBox), prevButton, FALSE, FALSE, 0);
> > +
> > +    GtkWidget *nextButton = gtk_button_new_from_stock(GTK_STOCK_GO_FORWARD);
> > +    gtk_box_pack_start(GTK_BOX(hBox), nextButton, FALSE, FALSE, 0);
> 
> Using stock icons for these is a bit confusing, you see a huge buttons saying Back/Forward, I would use buttons without labels and up/down icons instead of back/forward.

Done.

(In reply to comment #15)
> > Tools/MiniBrowser/gtk/BrowserSearchBar.c:182
> > +    gtk_widget_show(GTK_WIDGET(searchBar));
> 
> Forgot to mention that it would be nice if the search entry is focused when the search bar is shown

Done.

---

I've redesigned the UI. Now, the search bar is at the top with the entry in the middle and small up/down buttons and small close button at the right side.

The search entry has two icons to clear and to show a pop up menu with extended search options.

In addition, I've added several keyboard accelerators.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.