Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add a shortcut for editing images #1330

Merged
merged 1 commit into from
Aug 17, 2022
Merged

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Aug 17, 2022

Signed-off-by: szaimen szaimen@e.mail.de

@szaimen szaimen added enhancement New feature or request 3. to review Waiting for reviews labels Aug 17, 2022
@szaimen szaimen added this to the Nextcloud 25 milestone Aug 17, 2022
@szaimen szaimen requested a review from skjnldsv August 17, 2022 11:42
@szaimen
Copy link
Contributor Author

szaimen commented Aug 17, 2022

/compile amend /

Comment on lines 359 to +361
window.removeEventListener('keydown', this.keyboardDeleteFile)
window.removeEventListener('keydown', this.keyboardDownloadFile)
window.removeEventListener('keydown', this.keyboardEditFile)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be cleaner to have one method for the keyboardListener?

handleKeydown(event) {
event.stopImmediatePropagation()
// escape key
if (event.key === 'Escape') {
// Since we cannot call the closeMethod and know if there
// are unsaved changes, let's fake a close button trigger.
event.preventDefault()
document.querySelector('.FIE_topbar-close-button').click()
}
// ctrl + S = save
if (event.ctrlKey && event.key === 's') {
event.preventDefault()
document.querySelector('.FIE_topbar-save-button').click()
}
// ctrl + Z = undo
if (event.ctrlKey && event.key === 'z') {
event.preventDefault()
document.querySelector('.FIE_topbar-undo-button').click()
}
},

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably yes? Shall I change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both is fine...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, lgtm then :)

Signed-off-by: szaimen <szaimen@e.mail.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command nextcloud-command force-pushed the enh/noid/editing-shortcut branch from 5408bfc to eb2c6be Compare August 17, 2022 11:44
@skjnldsv skjnldsv merged commit 462c723 into master Aug 17, 2022
@skjnldsv skjnldsv deleted the enh/noid/editing-shortcut branch August 17, 2022 12:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants