Fix (image): Allow closing insert via url dialog window by enter key

[l-lejman]

Suggested merge commit message (convention)

Fix (image): Insert image via url dialog can be closed by "enter" button when input is focused. Closes #16902.

---

I tried to add better unit tests for this case but submitting form by "enter" button works different in browser than in unit test environment. I left comment to prevent confusion why unit test has no "enter" click.

[f1ames]

The approach for this fix went into a good direction, to have a form in place so enter could be handled 👍 However, the changes you applied affects DialogView component globally (see review comments) which is not what we want IMHO.

The right approach would be to modify to ImageInsertViaUrlUI to have the form element rendered into the dialog and then proper submit handling. There are other plugins (like MediaEmbed) which already have this so you may take a look how it's done there.

[f1ames]

DialogView is a "generic" component reused by CKEditor 5 in multiple places by multiple plugins. This change would affect all of the dialogs so this is not the right place for such change if we only want to adjust the behavior of a specific plugins dialog.

[f1ames]

It's a good approach to describe "not so obvious" code, however this could use some more details to be clear. You could describe that we fire form submit event to simulate what browser does on enter press instead of trying to trigger enter key event on a button directly.

[f1ames]

First thing, always make sure that CI is green when passing a PR for a review. Here, there is one job failing (cc is not 100% after recent changes).

Second thing, as for:

The approach for this fix went into a good direction, to have a form in place so enter could be handled 👍 However, the changes you applied affects DialogView component globally (see review comments) which is not what we want IMHO.

The right approach would be to modify to ImageInsertViaUrlUI to have the form element rendered into the dialog and then proper submit handling. There are other plugins (like MediaEmbed) which already have this so you may take a look how it's done there.

Well, you solved it by adding a flag, but it's unnecessary and makes the generic dialog more complex. As each plugin has a full control over it's views and can render anything there, there is no reason for lifting this logic higher in components hierarchy.
As suggested before, we already have plugins doing this. You may take a look on MediaEmbed plugin and its UI (here is the view definition) and see how "Image insert via URL" view can be adjusted (here).

[f1ames]

Suggested change

	/**
	* Browsers handle clicking Enter for form submitting by its own but unit test can not, it is known limitation.
	* We fire a form submit event to simulate the behavior that occurs when pressing Enter in the browser.
	*/
	/**
	* Browsers handle pressing Enter on forms natively by submitting it. We fire a form submit event to simulate that behavior.
	*/

[f1ames]

I think it could be a bit shorter as above. And a "nitpick", enter is pressed not clicked (usually at least 😄).

[f1ames]

Just wanted to add something here. The funny thing is that we already have a test which checks "enter submit" behavior, and it happily passes.

This is because, the tests simulates "submit via enter" by sending a submit event on a top dialog view element (which is a div):

ckeditor5/packages/ckeditor5-image/tests/imageinsert/imageinsertviaurlui.js

Lines 189 to 193 in fd1d41e

	testSubmit( 'form submit (enter key)', () => {
	const form = dialog.view.contentView.children.get( 0 );
	form.fire( 'submit' );
	} );

ckeditor5/packages/ckeditor5-image/src/imageinsert/ui/imageinserturlview.ts

Lines 71 to 72 in fd1d41e

	this.setTemplate( {
	tag: 'div',

And then we have submit handler attached to this view which catches this event and runs the callback:

ckeditor5/packages/ckeditor5-image/src/imageinsert/imageinsertviaurlui.ts

Lines 175 to 178 in fd1d41e

	if ( !this._formView ) {
	this._formView = this._createInsertUrlView();
	this._formView.on( 'submit', () => this._handleSave() );
	}

However, browser natively will not trigger the submit event when there is no form element 🤷 🙈

[f1ames]

LGTM 👍