search

asterics / WebACS

HTML5/JS version of the AsTeRICS Configuration Suite
Apache License 2.0
3 stars 2 forks source link

only use keydown and keyCode || which since it's implemented more con… #46

Closed klues closed 4 years ago

klues commented 4 years ago

…sistently across browsers, tested in Chrome, Firefox, Edge

fixes #44

sabicalija commented 4 years ago

Hi,

wäre es hier eventuell besser die key Property zu verwenden? https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values

keyCode dürfte deprecated sein. Oder? https://developer.mozilla.org/de/docs/Web/API/KeyboardEvent/keyCode

Falls die erste Variante wirklich besser ist, dann wird die Änderung nicht so schnell gehen, nehme ich an. Alle Plugins die etwas mit Keyboard machen funktionieren doch über keyCodes, oder?

Zusätzlich, wenn ich's richtig verstehe - und bitte korrigiere mich andernfalls - wird Support keyCode mit der Zeit wegfallen und es bleibt uns nichts anderes übrig als aus key zu wechseln.

klues commented 4 years ago

You are right, "keyCode" and "which" are depreceted, however they are older and therefore better supported by browsers, see https://caniuse.com/#search=KeyboardEvent.key and https://caniuse.com/#search=KeyboardEvent.which

So to my mind currently it's still better to use the old properties despite they are deprecated. In addition the bugfix was much easier to implement using these properties.

That's the question if "keyCode" and "which" will ever be removed, I don't really think so since way too much websites are relying on it and therefore a Browser would frustrate own users by doing so and therefore harming themselves without any benefit.

The most correct and future-proof approach would be to implement both, doing a main implementation with "key" and if "key" is not supported fallback to the current implementation with "keyCode/which". However this would be much more effort with no real benefit, so I did'nt do it.

sabicalija commented 4 years ago

Yes, I think implementing both is the most robust solution. And I agree. I don't think that Browser support will be removed in the near future.

However, wouldn't we have additional benefits in using keyCode? As far as I understood it, it should be platform-invariant. You can use the same Strings for all platforms (Win, Lin, Mac,...).

Additionally, we could use Strings for configuration, like "enter" and "shift" instead of cryptic numbers.

But, as a consequence, we would need to change the configuration of the Plugins using keyboard events and data, right?

klues commented 4 years ago

The whole discussion is only about the keyboard shortcuts in the GUI of WebACS (=Javascript). Any AsTeRICS-plugin-specific keycodes (=Java) are a completely different topic.

Before waiting for someone to make this really nice (parallel implementation) I'd just merge this PR, release it and maybe add a new enhancement-issue for beautifying this. This PR as-it-is massively improves UX and therefore should be published as soon as possible.

sabicalija commented 4 years ago

Sure. I didn't mean to stop you from integration. ;-)

Just wanted to discuss it a bit.

klues commented 4 years ago

Yeah, no problem. The possibility to discuss changes is the reason for PR's ;)

deinhofer commented 4 years ago

I successfully tested it on Firefox on an old Macbook (Mac Os X 10.9). Safari (7.0.1) did not work as the WebACS is not correctly layed out / visualized on that browser, but this is independent of this fix.

There is such one bad thing: The shortcuts use the Ctrl key and not the Mac key, which is typically used in other mac OS applications for such operations.

klues commented 4 years ago

Seems that it's possible to get the Mac key with event.metaKey. Could you try https://jsbin.com/huyifahuru/edit?js,output in Safari (and Firefox) on macOS?

If it works, I think it makes most sense to treat the Mac key as the same as the Ctrl key?! What do you think?

deinhofer commented 4 years ago

Seems that it's possible to get the Mac key with event.metaKey. Could you try https://jsbin.com/huyifahuru/edit?js,output in Safari (and Firefox) on macOS?

If it works, I think it makes most sense to treat the Mac key as the same as the Ctrl key?! What do you think? yes, it works. event.metaKey returns the mac key. Yes, I think treating it the same as the Ctrl key makes sense. Or do you know if on Mac it is typically used differently?

klues commented 4 years ago

tested also on Mac with new implementation using metaKey (mac key) -> works -> merged!