Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix block deleted when settings are opened #1090 #1109

Closed
wants to merge 1 commit into from
Closed

Fix block deleted when settings are opened #1090 #1109

wants to merge 1 commit into from

Conversation

alcarazzam
Copy link

Prevent block removal when settings are opened #1090

@neSpecc
Copy link
Member

neSpecc commented Apr 18, 2020

I think it would better to leave an ability to delete blocks this way. So it's better to close Block Settings in such a case.

@alcarazzam
Copy link
Author

I think it would better to leave an ability to delete blocks this way. So it's better to close Block Settings in such a case.

But there's a problem with input elements inside settings. If you press the backspace key while editing text in a settings' input, the block is deleted instead of doing the default behavior (remove a character).

@neSpecc
Copy link
Member

neSpecc commented Apr 18, 2020

I think it would better to leave an ability to delete blocks this way. So it's better to close Block Settings in such a case.

But there's a problem with input elements inside settings. If you press the backspace key while editing text in a settings' input, the block is deleted instead of doing the default behavior (remove a character).

Ok, got that. We need to think about how to detect if the Caret placed inside some kind of an editable element at the settings. Or maybe allow an ability to prevent default behavior from the plugin side.

@gohabereg
Copy link
Member

I think we can use document.activeElement for that

@alcarazzam
Copy link
Author

Thanks @neSpecc and @gohabereg for the suggestions. I'l try to implement the check if we're in an editable element tomorrow.

@alcarazzam
Copy link
Author

PR updated.

@@ -390,13 +390,23 @@ export default class UI extends Module {
private backspacePressed(event: KeyboardEvent): void {
const {BlockManager, BlockSelection, Caret} = this.Editor;

/** Prevent block removal while editing an input inside settings */
if (this.Editor.BlockSettings.opened) {
if (document.activeElement.tagName === 'INPUT') {
Copy link
Member

Choose a reason for hiding this comment

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

we have a useful method for that, see dom.canSetCaret()

Copy link
Author

@alcarazzam alcarazzam Apr 20, 2020

Choose a reason for hiding this comment

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

Ok, thanks. It has more features. I'll update the PR this evening if I have time.

@neSpecc
Copy link
Member

neSpecc commented Apr 25, 2020

The solution works, but it only allows to press backspace at the input. There are also some problems with Flipper - class responsible for tab/arrow navigation between buttons:

  • you can't navigate by arrows inside the input at the block tunes
  • you can't focus the input with Flipper
  • the input has no auto focus when block tunes are opened

@alcarazzam
Copy link
Author

Closing in favor of #1123

@alcarazzam alcarazzam closed this May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants