After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739638 - Use a popover for the hamburger menu
Use a popover for the hamburger menu
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
3.14.x (obsolete)
Other Linux
: Normal enhancement
: ---
Assigned To: Arnaud B.
Epiphany Maintainers
: 769625 (view as bug list)
Depends on: 735487 735727
Blocks: 755524
 
 
Reported: 2014-11-04 19:17 UTC by Michael Catanzaro
Modified: 2016-09-22 22:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
web-app-utils: Do not copy cookies when creating new web app (3.23 KB, patch)
2016-09-22 21:51 UTC, Iulian Radu
none Details | Review
Oops, sorry for the mistake above. This implementation is for the mockup posted by (3.23 KB, patch)
2016-09-22 21:53 UTC, Iulian Radu
none Details | Review
Implement new Page Menu popover design (38.57 KB, patch)
2016-09-22 21:57 UTC, Iulian Radu
none Details | Review
Implement new Page Menu popover design (38.69 KB, application/mbox)
2016-09-22 22:13 UTC, Iulian Radu
  Details

Description Michael Catanzaro 2014-11-04 19:17:14 UTC
"A header bar menu is contained within a popover." [1] Let's replace the traditional dropdown menu with a modern popover.

In particular, this means we need to prune some items from the menu to make room:

* New Tab -- this is redundant with the button in the header bar
* Undo, redo, cut, copy, paste -- "Header bar menus are not a good choice for performing actions on selected content: when content hasn’t been selected, the menu will contain unhelpful insensitive menu items, and when content has been selected, possible actions will not be advertised."
* Close -- "Header bar menus shouldn’t include a close menu item, since this action is already provided by the header bar. It can also be ambiguous as to what a close menu item refers to." We might rename it to Close Tab (since this is what it does), but I don't think it's necessary as it's redundant with the x button on the tabs.

Some of the other menu items may optionally be modified to use other UI elements. For example, gedit has buttons for Refresh, Print, and Fullscreen; our menu is missing Fullscreen and needs to add it somehow, and a Print button would work fine for us, although we don't want Refresh in that menu.  Christian suggested using a slider with tick marks to control the zoom.

My primary concern is that the bookmarks submenu might not translate nicely to a popover, and may require extra effort.

[1] https://developer.gnome.org/hig/stable/header-bar-menus.html.en
Comment 1 Yosef Or Boczko 2014-11-04 19:19:27 UTC
Part of this look like a duplicate of bug 735727?
Comment 2 Michael Catanzaro 2014-11-04 22:57:27 UTC
(In reply to comment #1)
> Part of this look like a duplicate of bug 735727?

Certainly a bug we'll need to sort out at the same time.
Comment 3 Michael Catanzaro 2016-08-08 14:21:52 UTC
*** Bug 769625 has been marked as a duplicate of this bug. ***
Comment 4 Michael Catanzaro 2016-08-08 14:22:21 UTC
Hamburger menu mockup at the bottom here: https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/web/web-bookmarks-and-header-bar-wires.png
Comment 5 Iulian Radu 2016-09-22 21:51:58 UTC
Created attachment 336109 [details] [review]
web-app-utils: Do not copy cookies when creating new web app

It breaks Google Inbox, which expects cookies to be present from
other Google domains if cookies are also present for Inbox.

https://bugzilla.gnome.org/show_bug.cgi?id=771540
Comment 6 Iulian Radu 2016-09-22 21:53:55 UTC
Created attachment 336110 [details] [review]
Oops, sorry for the mistake above. This implementation is for the mockup posted by

Michael.
Comment 7 Iulian Radu 2016-09-22 21:57:43 UTC
Created attachment 336111 [details] [review]
Implement new Page Menu popover design

I messed up twice. Sorry! This one is the patch I was trying to post.
Comment 8 Michael Catanzaro 2016-09-22 22:02:51 UTC
Review of attachment 336111 [details] [review]:

::: src/ephy-header-bar.c
@@ +644,3 @@
                         gtk_image_new_from_icon_name ("open-menu-symbolic", GTK_ICON_SIZE_BUTTON));
   gtk_widget_set_valign (button, GTK_ALIGN_CENTER);
+  g_type_ensure(G_TYPE_THEMED_ICON);

Wow, impressed that you figured out you needed this. Do you know why? I'm curious.

Also: missing space before opening parentheses

::: src/ephy-window.c
@@ +1027,3 @@
+  /* Update the zoom level entry in the page menu popover:
+   * - obtain the popover from the page menu button
+   * - obtain the box (the first children of the popover)

children -> child
Comment 9 Iulian Radu 2016-09-22 22:13:01 UTC
Created attachment 336112 [details]
Implement new Page Menu popover design

(In reply to Michael Catanzaro from comment #8)

> Wow, impressed that you figured out you needed this. Do you know why? I'm
> curious.
> 
> Also: missing space before opening parentheses

Tim suggested that I should use that one. I guess we're not using GThemedIcon anywhere else in code so _get_type() has not been called yet and the type has not been registered with the type system. g_type_ensure() takes care of that.

> 
> ::: src/ephy-window.c
> @@ +1027,3 @@
> +  /* Update the zoom level entry in the page menu popover:
> +   * - obtain the popover from the page menu button
> +   * - obtain the box (the first children of the popover)
> 
> children -> child

Oops.