menpo / landmarker.io

Image and mesh annotation web application
https://www.landmarker.io
BSD 3-Clause "New" or "Revised" License
114 stars 21 forks source link

Ctrl+S doesn't work on Windows #108

Closed csagonas closed 8 years ago

csagonas commented 8 years ago

Notes by @jabooth:

Unfortunately on Chrome Windows, Ctrl + S brings up the save webpage dialog, so we can't actually save with the keyboard. Maybe we should just do S? How does something like gmail get around these kind of cross-platform issues?

lirsacc commented 8 years ago

We use ctrl to avoid unwanted save as it is reflected on the backend. I feel like just S is too prone to accidental saves.

I think we don't use preventDefault on this one though, so it may be a simple fix, but I don't have access to a windows laptop to confirm.

EDIT: See branch https://github.com/menpo/landmarker.io/tree/keyboard-issues where I moved it to the keydown handler and successfully bypass the dialog on OS X, if anyone can confirm that it works on win we can go ahead and merge

jabooth commented 8 years ago

@lirsacc I can try this on Windows tonight.

jabooth commented 8 years ago

@lirsacc confirmed, that works as a fix.

Can you make a PR for the branch and just reference this issue? Now we are incrementally I'd rather have individual PRs to point to for history if possible.

Once that is merged, we can close this issue, and do a patch release.

lirsacc commented 8 years ago

Linked to #110 as the refactor encompassed both. Unless it's urgent I'd wait for both to be cleared.

jabooth commented 8 years ago

No problem, we'll wait for #110 to get resolved first.

jabooth commented 8 years ago

Solved by #122