Don't overwrite browser zoom shortcuts

[fjellfly]

Some browsers enable the user to set the zoomlevel of the website using Ctrl and + or -. The default version of KeyboardZoom catches these keydown events and adjusts the map zoomlevel. There might be a situation where a user wants to increase or decrease the size of e.g. the controls as well. They are not able to do this via shortcuts because they are handled by OpenLayers. To get around this, I suggest that the default KeyboardZoom should only adjust the zoom level if + or - was pressed without Ctrl being pressed at the same time. This way, the event is raised to the browser if Ctrl was pressed, too.

[ahocevar]

Thanks for the contribution! I‘d suggest fixing this directly in KeybordZoom.js, because it‘s the documented and expected behavior.

[fjellfly]

I‘d suggest fixing this directly in KeybordZoom.js, because it‘s the documented and expected behavior.

True - I adjusted the code according to your suggestion.

[github-actions]

📦 Preview the website for this branch here: https://deploy-preview-14946--ol-site.netlify.app/.

[mike-000]

Many keyboard layouts use the shift key to access +. Using noModifierKeys alone will prevent that working. It will need a custom function such as

 function (mapBrowserEvent) {
      return (
        (noModifierKeys(mapBrowserEvent) || shiftKeyOnly(mapBrowserEvent)) && targetNotEditable(mapBrowserEvent)
      );
 }

[fjellfly]

Many keyboard layouts use the shift key to access +.

That's a good point! I extended the filter criteria to allow shift. I did this using a custom function as you suggested. I had the feeling that it could also be an option to add another ol/events/condition like noPlatformModifierKey. However, as this is my first PR to the project I wasn't sure if you prefer not to have such an additional Condition if it's only used at a single location in the code.

[ahocevar]

Why not simply

Suggested change

	: function (mapBrowserEvent) {
	return (
	(noModifierKeys(mapBrowserEvent) ||
	shiftKeyOnly(mapBrowserEvent)) &&
	targetNotEditable(mapBrowserEvent)
	);
	};
	: (mapBrowserEvent) => !platformModifierKeyOnly(mapBrowserEvent) && targetNotEditable(mapBrowserEvent)

?

[mike-000]

On keyboards which need shift to type + browser zoom needs ctrl shift.

[mike-000]

(although on Windows Chrome with UK keyboard ctrl = will also activate browser zoom).

[ahocevar]

@fjellfly I think having a platformModifierKey condition would make sense, which returns true when the Ctrl (or Command on Mac) modifier key is pressed.

[fjellfly]

@fjellfly I think having a platformModifierKey condition would make sense, which returns true when the Ctrl (or Command on Mac) modifier key is pressed.

@ahocevar Okay, I added a platformModifierKey condition that is used to prevent zoom if ctrl/meta is pressed.

[ahocevar]

Thanks, @fjellfly, for this excellent first-time contribution.

[ahocevar]

@fjellfly There is a minor issue with the new test: on a Mac, it will fail. Instead of testing with the Ctrl key hard coded, the Ctrl or Cmd key should be used, depending on the platform. Would be great if you could create a small follow-up pull request to fix that.

[fjellfly]

@fjellfly There is a minor issue with the new test: on a Mac, it will fail. Instead of testing with the Ctrl key hard coded, the Ctrl or Cmd key should be used, depending on the platform. Would be great if you could create a small follow-up pull request to fix that.

@ahocevar Oh, of course, here is a fix for that.