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 763948 - Add Control+= as a mnemonic to zoom in the view
Add Control+= as a mnemonic to zoom in the view
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Views: All
3.20.x
Other Linux
: Normal minor
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-20 17:44 UTC by Mohammed Sadiq
Modified: 2016-06-24 18:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
files-view: obsolete restoring default zoom level (8.95 KB, patch)
2016-06-09 09:25 UTC, Ernestas Kulik
none Details | Review
files-view: add ctrl-= as "zoom in" accelerator (1.65 KB, patch)
2016-06-09 09:25 UTC, Ernestas Kulik
none Details | Review
application: add nautilus_application_set_accelerators() (5.03 KB, patch)
2016-06-09 14:34 UTC, Ernestas Kulik
none Details | Review
application: rename nautilus_application_add_accelerator() (12.47 KB, patch)
2016-06-09 14:35 UTC, Ernestas Kulik
none Details | Review
Rebased patches on top of Neil’s work (9.90 KB, patch)
2016-06-20 14:42 UTC, Ernestas Kulik
none Details | Review
files-view: add ctrl-= as "zoom in" accelerator (1.65 KB, patch)
2016-06-20 14:43 UTC, Ernestas Kulik
none Details | Review
application: add nautilus_application_set_accelerators() (5.03 KB, patch)
2016-06-20 14:43 UTC, Ernestas Kulik
none Details | Review
application: rename nautilus_application_add_accelerator() (12.48 KB, patch)
2016-06-20 14:44 UTC, Ernestas Kulik
none Details | Review
files-view: obsolete restoring default zoom level (9.16 KB, patch)
2016-06-24 08:57 UTC, Ernestas Kulik
committed Details | Review
files-view: add ctrl-= as "zoom in" accelerator (1.65 KB, patch)
2016-06-24 08:57 UTC, Ernestas Kulik
committed Details | Review
application: add nautilus_application_set_accelerators() (4.19 KB, patch)
2016-06-24 08:57 UTC, Ernestas Kulik
committed Details | Review
application: rename nautilus_application_add_accelerator() (12.43 KB, patch)
2016-06-24 08:57 UTC, Ernestas Kulik
committed Details | Review
files-view: add accel for "zoom-standard" action (1.20 KB, patch)
2016-06-24 08:57 UTC, Ernestas Kulik
committed Details | Review

Description Mohammed Sadiq 2016-03-20 17:44:16 UTC
Currently Control+shif+= is required to zoom in the view. As control+= is common (Eg: Firefox) for zoom in, It would be nice to have that.
Comment 1 Carlos Soriano 2016-03-21 09:33:25 UTC
I believe this is done in order to be comfortable with the US keyboard layout, since to me "=" would make sense to reset to default, not to increase the zoom. That's why we use + and -.

But imho we cannot add a keyboard shortcut that is comfortable for all layouts if the key used are special signs/etc.

The only keys that I believe are accessible without shift on all layouts are numbers and letters only.

So not sure about this addition that could prevent us to change the current shortcut of resetting the zoom <ctrl>0 to <ctrl>= or something like that.

Something to think about.
Comment 2 Mohammed Sadiq 2016-03-21 18:58:04 UTC
(In reply to Carlos Soriano from comment #1)

> So not sure about this addition that could prevent us to change the current
> shortcut of resetting the zoom <ctrl>0 to <ctrl>= or something like that.

Using <ctrl>= for resetting zoom will break the workflow of several people.
I know no application that I uses to use <ctrl>= for resetting zoom.

> 
> Something to think about.

Let me note some applications:

Gnome Web (epiphany), eog, Firefox:
  Ctrl-0: reset zoom
  Ctrl-+/Ctrl-= : zoom in 
  Ctrl-- : zoom out

inkscape:
  =/+ : zoom in
  -/_ : zoom out

evince:
  =/+/Ctrl-+/Ctrl-= : zoom in
  -/Ctrl-- : zoom out

GNU Emacs:
  C-x C-=/C-x C-+ : zoom in
  C-x C-- : zoom out
  C-x C-0 : reset zoom
  (or by using =/+,-, or 0 just after any of one above)
Comment 3 Carlos Soriano 2016-03-22 09:08:10 UTC
(In reply to Mohammed Sadiq from comment #2)
> (In reply to Carlos Soriano from comment #1)
> 
> > So not sure about this addition that could prevent us to change the current
> > shortcut of resetting the zoom <ctrl>0 to <ctrl>= or something like that.
> 
> Using <ctrl>= for resetting zoom will break the workflow of several people.
> I know no application that I uses to use <ctrl>= for resetting zoom.

Not that it makes sense to me, but I agree it can break workflows if there is enough apps using that shortcut for a single different thing.

> 
> > 
> > Something to think about.
> 
> Let me note some applications:
> 
> Gnome Web (epiphany), eog, Firefox:
>   Ctrl-0: reset zoom
>   Ctrl-+/Ctrl-= : zoom in 
>   Ctrl-- : zoom out
> 
> inkscape:
>   =/+ : zoom in
>   -/_ : zoom out
> 
> evince:
>   =/+/Ctrl-+/Ctrl-= : zoom in
>   -/Ctrl-- : zoom out
> 
> GNU Emacs:
>   C-x C-=/C-x C-+ : zoom in
>   C-x C-- : zoom out
>   C-x C-0 : reset zoom
>   (or by using =/+,-, or 0 just after any of one above)

Fair enough, seems it's common. Thanks for gathering the info.
Comment 4 Ernestas Kulik 2016-05-07 11:51:11 UTC
The issue with resetting the zoom level is that it doesn’t work.

The default zoom level is set both when manipulating zoom_level_scale and using the accelerators. This makes the view reset the zoom level to the current one.

I’m taking a look at this at the moment, but I’m not sure how to go about fixing this yet.
Comment 5 Carlos Soriano 2016-05-09 11:21:29 UTC
(In reply to Ernestas Kulik from comment #4)
> The issue with resetting the zoom level is that it doesn’t work.
> 
> The default zoom level is set both when manipulating zoom_level_scale and
> using the accelerators. This makes the view reset the zoom level to the
> current one.
> 
> I’m taking a look at this at the moment, but I’m not sure how to go about
> fixing this yet.

Right, there is no reset to default anymore, since the preferences is always what the user chooses for.

Maybe we can change this semantic to change to the "standard" level.
Comment 6 Georges Basile Stavracas Neto 2016-05-29 01:17:42 UTC
Looks like the common way to reset to default is <Ctrl> 0 (Zero). It seems like this is a stabilished standard, breaking it may not be a wise idea.
Comment 7 Carlos Soriano 2016-05-30 08:42:09 UTC
(In reply to Georges Basile Stavracas Neto from comment #6)
> Looks like the common way to reset to default is <Ctrl> 0 (Zero). It seems
> like this is a stabilished standard, breaking it may not be a wise idea.

Hey Georges,

What do you mean exactly? Where is this breaking?
Comment 8 Georges Basile Stavracas Neto 2016-05-31 14:47:00 UTC
(In reply to Carlos Soriano from comment #7)
> What do you mean exactly? Where is this breaking?

I mean "let's not change the standard behavior" :)
Comment 9 Carlos Soriano 2016-05-31 15:04:29 UTC
(In reply to Georges Basile Stavracas Neto from comment #8)
> (In reply to Carlos Soriano from comment #7)
> > What do you mean exactly? Where is this breaking?
> 
> I mean "let's not change the standard behavior" :)

The "standard behaviour" is not present in nautilus right now and cannot be present because there not such a "default size" other than the user selected current one.

I'm still unsure what change or break you are referring to.
Comment 10 Ernestas Kulik 2016-06-08 18:11:55 UTC
Since no one else is taking this: I noticed that nautilus_application_add_accelerator() sets, rather than adds accelerators. So having multiple accels for an action is impossible without modifying the function.

Some layouts have ‘+’ on the first level and it would be beneficial to support both ‘+’ and ‘=’.

I have the code ready, but want to know if I should bother with the patch.

Also, what Georges meant was breaking the de facto-standard behavior (i.e. resetting to the initial zoom level).
Comment 11 Carlos Soriano 2016-06-09 07:54:50 UTC
(In reply to Ernestas Kulik from comment #10)
> Since no one else is taking this: I noticed that
> nautilus_application_add_accelerator() sets, rather than adds accelerators.
> So having multiple accels for an action is impossible without modifying the
> function.

It's intended, I was trying to avoid precisely having multiple accelerators for the same pourpose. But already had to blend my arm with this in a few occasions :) (files-view has some examples).

> 
> Some layouts have ‘+’ on the first level and it would be beneficial to
> support both ‘+’ and ‘=’.

Yeah, still... sounds fishy to me. But I agree is fine since it's used in other apps.

> 
> I have the code ready, but want to know if I should bother with the patch.
> 
> Also, what Georges meant was breaking the de facto-standard behavior (i.e.
> resetting to the initial zoom level).

I think I'm missing something :/ There is no default zoom in Nautilus apart of the current one. So "default zoom" is pointless in Nautilus right now, and the same to have a shortcut for it.
Comment 12 Ernestas Kulik 2016-06-09 08:20:42 UTC
(In reply to Carlos Soriano from comment #11)
> I think I'm missing something :/ There is no default zoom in Nautilus apart
> of the current one. So "default zoom" is pointless in Nautilus right now,
> and the same to have a shortcut for it.

It might create confusion for new users, as a number of programs in the wild exhibit the previous behavior.

I agree that this might not be relevant in Nautilus, because there’s only a handful of zoom levels (unlike in a, let’s say, web browser or a document reader, where you often can enter a custom value and/or have a great enough number of presets) and the user can easily tell what the starting one was.

Using ctrl-= might piss off German et al. users and ctrl-+ is something already well established in Nautilus (it also works in other programs), but US and similar layouts are at a disadvantage, because ‘+’ is only accessible with a second modifier.
Comment 13 Carlos Soriano 2016-06-09 08:28:18 UTC
Are you proposing to change the current Nautilus behaviour where the current setting is always the default one so "reseting to a default zoom" would make sense?

Note that this change was precisely introduced because was confusing to users to change the zoom and realize that the default one was not changed (the option to change the default one was in preferences).
Comment 14 Ernestas Kulik 2016-06-09 09:25:07 UTC
Created attachment 329442 [details] [review]
files-view: obsolete restoring default zoom level

Restoring to the default zoom level does not work anymore, as Nautilus
sets the default level to the current one. This commit removes the
restore functionality.
Comment 15 Ernestas Kulik 2016-06-09 09:25:27 UTC
Created attachment 329443 [details] [review]
files-view: add ctrl-= as "zoom in" accelerator

Both ctrl-= and ctrl-+ are commonly used for zooming in for
consistency, as some keyboard layouts require pressing another modifier
to access the plus symbol. This commit adds ctrl-= as another
accelerator for zooming in.
Comment 16 Ernestas Kulik 2016-06-09 14:34:23 UTC
Created attachment 329468 [details] [review]
application: add nautilus_application_set_accelerators()

nautilus_application_add_accelerator() only allows setting a single
accelerator, which in some cases is not enough. This commit adds a
wrapper for gtk_application_set_accels_for_action(), which takes an
array of accels.
Comment 17 Ernestas Kulik 2016-06-09 14:35:43 UTC
Created attachment 329469 [details] [review]
application: rename nautilus_application_add_accelerator()

nautilus_application_add_accelerator() is ambiguous in that it conveys
that the function appends an accelerator to the list. This commit
removes the ambiguity by renaming the function accordingly.
Comment 18 Ernestas Kulik 2016-06-20 13:11:59 UTC
Hey, Carlos, any update on this?
Comment 19 Ernestas Kulik 2016-06-20 14:42:56 UTC
Created attachment 330073 [details] [review]
Rebased patches on top of Neil’s work
Comment 20 Ernestas Kulik 2016-06-20 14:43:34 UTC
Created attachment 330074 [details] [review]
files-view: add ctrl-= as "zoom in" accelerator

Both ctrl-= and ctrl-+ are commonly used for zooming in for
consistency, as some keyboard layouts require pressing another modifier
to access the plus symbol. This commit adds ctrl-= as another
accelerator for zooming in.
Comment 21 Ernestas Kulik 2016-06-20 14:43:46 UTC
Created attachment 330075 [details] [review]
application: add nautilus_application_set_accelerators()

nautilus_application_add_accelerator() only allows setting a single
accelerator, which in some cases is not enough. This commit adds a
wrapper for gtk_application_set_accels_for_action(), which takes an
array of accels.
Comment 22 Ernestas Kulik 2016-06-20 14:44:00 UTC
Created attachment 330076 [details] [review]
application: rename nautilus_application_add_accelerator()

nautilus_application_add_accelerator() is ambiguous in that it conveys
that the function appends an accelerator to the list. This commit
removes the ambiguity by renaming the function accordingly.
Comment 23 Carlos Soriano 2016-06-20 21:56:54 UTC
Review of attachment 330073 [details] [review]:

yep, commit after Neil's work is merged.

::: src/nautilus-canvas-view.c
@@ +799,3 @@
 }
 
+

spurious line
Comment 24 Carlos Soriano 2016-06-20 21:57:23 UTC
Review of attachment 330074 [details] [review]:

Comment 25 Carlos Soriano 2016-06-20 21:58:24 UTC
Review of attachment 330075 [details] [review]:

Comment 26 Carlos Soriano 2016-06-20 21:59:04 UTC
Review of attachment 330076 [details] [review]:

LGTM thanks!
Comment 27 Ernestas Kulik 2016-06-24 08:57:06 UTC
Created attachment 330299 [details] [review]
files-view: obsolete restoring default zoom level

Restoring to the default zoom level does not work anymore, as Nautilus
sets the default level to the current one. This commit removes the
restore functionality.
Comment 28 Ernestas Kulik 2016-06-24 08:57:17 UTC
Created attachment 330300 [details] [review]
files-view: add ctrl-= as "zoom in" accelerator

Both ctrl-= and ctrl-+ are commonly used for zooming in for
consistency, as some keyboard layouts require pressing another modifier
to access the plus symbol. This commit adds ctrl-= as another
accelerator for zooming in.
Comment 29 Ernestas Kulik 2016-06-24 08:57:26 UTC
Created attachment 330301 [details] [review]
application: add nautilus_application_set_accelerators()

nautilus_application_add_accelerator() only allows setting a single
accelerator, which in some cases is not enough. This commit adds a
wrapper for gtk_application_set_accels_for_action(), which takes an
array of accels.
Comment 30 Ernestas Kulik 2016-06-24 08:57:37 UTC
Created attachment 330302 [details] [review]
application: rename nautilus_application_add_accelerator()

nautilus_application_add_accelerator() is ambiguous in that it conveys
that the function appends an accelerator to the list. This commit
removes the ambiguity by renaming the function accordingly.
Comment 31 Ernestas Kulik 2016-06-24 08:57:54 UTC
Created attachment 330303 [details] [review]
files-view: add accel for "zoom-standard" action

An action for setting the standard zoom level has been added, but it has
no accelerators. This commit adds one.
Comment 32 Carlos Soriano 2016-06-24 11:04:28 UTC
Review of attachment 330299 [details] [review]:

good
Comment 33 Carlos Soriano 2016-06-24 11:04:50 UTC
Review of attachment 330300 [details] [review]:

LGTM
Comment 34 Carlos Soriano 2016-06-24 11:05:43 UTC
Review of attachment 330301 [details] [review]:

LGTM
Comment 35 Carlos Soriano 2016-06-24 11:06:22 UTC
Review of attachment 330302 [details] [review]:

yep!
Comment 36 Carlos Soriano 2016-06-24 11:07:24 UTC
Review of attachment 330303 [details] [review]:

excellent, thanks!!

Of course wait until Neil's work is in master. But I think it's going to be today or tomorrow :)
Comment 37 Ernestas Kulik 2016-06-24 17:59:41 UTC
Attachment 330299 [details] pushed as b8e9778 - files-view: obsolete restoring default zoom level
Attachment 330300 [details] pushed as 60e487c - files-view: add ctrl-= as "zoom in" accelerator
Attachment 330301 [details] pushed as 1393866 - application: add nautilus_application_set_accelerators()
Attachment 330302 [details] pushed as d89c92a - application: rename nautilus_application_add_accelerator()
Attachment 330303 [details] pushed as e13c75e - files-view: add accel for "zoom-standard" action