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 764368 - Allow vertical tabs and more control about the tabs placement.
Allow vertical tabs and more control about the tabs placement.
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Tabs
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks: 755388
 
 
Reported: 2016-03-30 14:00 UTC by Carlos Alberto Lopez Perez
Modified: 2016-04-04 18:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch implemeting the feature (5.61 KB, patch)
2016-03-30 14:03 UTC, Carlos Alberto Lopez Perez
accepted-commit_now Details | Review
v2 patch addresing reviewer comments (5.58 KB, patch)
2016-03-30 15:48 UTC, Carlos Alberto Lopez Perez
none Details | Review
Allow vertical tabs and more control about the tabs placement. (5.59 KB, patch)
2016-03-30 17:03 UTC, Michael Catanzaro
none Details | Review
Allow vertical tabs and more control about the tabs placement (5.62 KB, patch)
2016-04-03 22:26 UTC, Michael Catanzaro
committed Details | Review
notebook: Allow changing new tab settings at runtime (3.38 KB, patch)
2016-04-03 22:26 UTC, Michael Catanzaro
committed Details | Review

Description Carlos Alberto Lopez Perez 2016-03-30 14:00:23 UTC
The current way epiphany handles tabs (the tab bar at the top and expanding the tabs size to fill the tab bar) is a bit inconvenient when you have more than 8 tabs open, as is not easy to switch between tabs.

I (and other power users) have the habit of opening any link on new tabs (with the wheel click of the mouse) to let the new page load in the background meanwhile you continue reading the main article.

Previously there was an extension that allowed to configure vertical tabs in epiphany <https://chris-lamb.co.uk/projects/epiphany-tabs-right> but since we removed support for epiphany extensions this is not longer possible.
Comment 1 Carlos Alberto Lopez Perez 2016-03-30 14:03:44 UTC
Created attachment 325015 [details] [review]
Patch implemeting the feature

I'm proposing here a simple patch that adds two new settings that allow to configure vertical tabs and other layout for the tabs. For example, it also allows to keep tabs on the top but disable the auto filling feature (that will be useful for example for users annoyed by bug 720749).

Keeping the auto filling feature on meanwhile configuring vertical tabs can be also useful for touch device where the total surface for clicking on a tab makes it easier to use.

This new settings are not exposed on the epiphany preferences windows, they should be configured via gsettings.

The default behaviour is not changed. The default values for this new settings are to keep the current way of handling tabs.
Comment 2 Michael Catanzaro 2016-03-30 15:25:30 UTC
Review of attachment 325015 [details] [review]:

Yeah, our designers didn't like vertical tabs, but no reason we can't have it as a hidden option.

And I'll be thinking about whether it makes sense to turn off tab expansion by default.

::: src/ephy-notebook.c
@@ +402,3 @@
+ephy_settings_get_tabs_bar_position (void)
+{
+   EphyPrefsUITabsBarPosition position;

Use two-space indentation.

@@ +407,3 @@
+
+   switch (position) {
+    case EPHY_PREFS_UI_TABS_BAR_POSITION_TOP:    return GTK_POS_TOP;

Please put the return statements on the next line in new code.

Also, please fix these warnings:

ephy-notebook.c: In function ‘ephy_settings_get_tabs_bar_position’:
ephy-notebook.c:408:4: error: switch missing default case [-Werror=switch-default]
    switch (position) {
    ^
ephy-notebook.c:414:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors


Consider building Epiphany with --enable-Werror when you're developing it, to catch these.

@@ +696,3 @@
                            GTK_WIDGET (tab_widget),
+                           "tab-expand", g_settings_get_boolean
+                                           (EPHY_SETTINGS_UI,

I kinda prefer to have a longer line, with the first parameter on the same line instead of a new line.
Comment 3 Carlos Alberto Lopez Perez 2016-03-30 15:29:40 UTC
(In reply to Michael Catanzaro from comment #2)
> Also, please fix these warnings:
> 
> ephy-notebook.c: In function ‘ephy_settings_get_tabs_bar_position’:
> ephy-notebook.c:408:4: error: switch missing default case
> [-Werror=switch-default]
>     switch (position) {
>     ^
> ephy-notebook.c:414:1: error: control reaches end of non-void function
> [-Werror=return-type]
>  }
>  ^
> cc1: all warnings being treated as errors
> 

Clang didn't printed that warning. The warning is bogus, because there is no way that such thing could happen, but I can add a default case to the switch to silent the warning with gcc.
Comment 4 Carlos Alberto Lopez Perez 2016-03-30 15:48:21 UTC
Created attachment 325031 [details] [review]
v2 patch addresing reviewer comments
Comment 5 Michael Catanzaro 2016-03-30 17:03:54 UTC
Created attachment 325034 [details] [review]
Allow vertical tabs and more control about the tabs placement.

  * This adds two new settings to org.gnome.Epiphany.ui
    - expand-tabs-bar controls whether the tabs should fill the
      available space in the bar.
    - tabs-bar-position controls the position of the tabs bar
      (GtkNotebook)

  * The default value is to keep the current UI (tabs bar on the top
    and expand the tabs to fill the space).

  * To configure vertical tabs, tabs-bar-position can be set to left
    and expand-tabs-bar to false. Setting expand-tabs-bar to true
    when tabs-bar-position is left or right can be also useful
    for touch devices.
Comment 6 Michael Catanzaro 2016-03-30 17:06:32 UTC
Would be better if the notebook would respond to setting changes at runtime, but I think it's fine as-is.
Comment 7 Michael Catanzaro 2016-03-31 13:36:43 UTC
(In reply to Michael Catanzaro from comment #6)
> Would be better if the notebook would respond to setting changes at runtime,
> but I think it's fine as-is.

On further thought, I think we should try to implement this. And I actually don't see the expand-tabs-bar setting working at all. :/
Comment 8 Carlos Alberto Lopez Perez 2016-03-31 13:45:33 UTC
(In reply to Michael Catanzaro from comment #7)
> (In reply to Michael Catanzaro from comment #6)
> > Would be better if the notebook would respond to setting changes at runtime,
> > but I think it's fine as-is.
> 
> On further thought, I think we should try to implement this. 

Please, can we first land this and then open another bug for that improvement? 

> And I actually
> don't see the expand-tabs-bar setting working at all. :/

Is working for me. I can attach screenshots if you wish.

You have to restart the browser after the changes.
Check that the setting has changed by running

gsettings get org.gnome.Epiphany.ui expand-tabs-bar

before starting epiphany
Comment 9 Carlos Alberto Lopez Perez 2016-03-31 13:56:27 UTC
(In reply to Carlos Alberto Lopez Perez from comment #8)
> You have to restart the browser after the changes.

If you have more than one epiphany process you have to close them all.

pkill -9 epiphany

Before opening a new one with the new settings
Comment 10 Michael Catanzaro 2016-04-03 21:19:51 UTC
(In reply to Michael Catanzaro from comment #7)
> On further thought, I think we should try to implement this. And I actually
> don't see the expand-tabs-bar setting working at all. :/

Must have tested it wrong. It's working now. It responds a bit wonky to runtime changes though; you can wind up wind some expanded and some non-expendad tabs.
Comment 11 Michael Catanzaro 2016-04-03 22:26:54 UTC
Created attachment 325283 [details] [review]
Allow vertical tabs and more control about the tabs placement

  * This adds two new settings to org.gnome.Epiphany.ui
    - expand-tabs-bar controls whether the tabs should fill the
      available space in the bar.
    - tabs-bar-position controls the position of the tabs bar
      (GtkNotebook)

  * The default value is to keep the current UI (tabs bar on the top
    and expand the tabs to fill the space).

  * To configure vertical tabs, tabs-bar-position can be set to left
    and expand-tabs-bar to false. Setting expand-tabs-bar to true
    when tabs-bar-position is left or right can be also useful
    for touch devices.
Comment 12 Michael Catanzaro 2016-04-03 22:26:57 UTC
Created attachment 325284 [details] [review]
notebook: Allow changing new tab settings at runtime
Comment 13 Michael Catanzaro 2016-04-03 22:29:37 UTC
I changed the code style in your patch a bit (including using g_assert_not_reached in the switch statement); if you approve, then I'll push this.
Comment 14 Carlos Alberto Lopez Perez 2016-04-03 22:46:22 UTC
(In reply to Michael Catanzaro from comment #13)
> I changed the code style in your patch a bit (including using
> g_assert_not_reached in the switch statement); if you approve, then I'll
> push this.

Looks good to me. Thanks!
Comment 15 Michael Catanzaro 2016-04-04 14:38:20 UTC
Attachment 325283 [details] pushed as 1c7aee0 - Allow vertical tabs and more control about the tabs placement
Attachment 325284 [details] pushed as fa2b175 - notebook: Allow changing new tab settings at runtime