Skip to content

Completer.js: Ensure autosubmit is triggered if no suggestion is selected #355

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.

Sign up for GitHub

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

Open
raviks789 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Completer.js: Ensure autosubmit is triggered if no suggestion is selected #355

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6f0206
Completer.js: Trigger autosubmit on focus out for manual input if the…
raviks789 Mar 4, 2026
ee71399
fix: resolve review comments
raviks789 Mar 6, 2026
a195165
fix: autosubmit only if the suggestion is closed
raviks789 Mar 6, 2026
18750a2
Completer: Move autosubmit logic into the timer `onFocusOut`
raviks789 Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions asset/js/widget/Completer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ define(["../notjQuery"], function ($) {
constructor(input, instrumented = false) {
this.input = input;
this.instrumented = instrumented;
this.hasBeenManuallyChanged = false; // Flag to identify if the input has been manually changed.
this.selectionStartInput = null;
this.selectionActive = false;
this.mouseSelectionActive = false;
Expand Down Expand Up @@ -268,6 +269,8 @@ define(["../notjQuery"], function ($) {

complete(input, value, data) {
$(input).focus({ scripted: true });
// Disable autosubmit for the input in non-instrumented mode if the input is completed.
this.hasBeenManuallyChanged = false;

if (this.instrumented) {
if (! Object.keys(data).length) {
Expand Down Expand Up @@ -321,8 +324,12 @@ define(["../notjQuery"], function ($) {

if (! stopAtEdge && this.completedValue !== null) {
if (input === this.completedInput) {
// Re-enable autosubmit for the input in non-instrumented mode when the user moves from suggestions back to the input.
this.hasBeenManuallyChanged = true;
this.suggest(this.completedInput, this.completedValue);
} else {
// Disable the autosubmit for the input in non-instrumented mode while moving through suggestions.
this.hasBeenManuallyChanged = false;
this.suggest(this.completedInput, input.value, { ...input.dataset });
}
}
Expand Down Expand Up @@ -476,6 +483,23 @@ define(["../notjQuery"], function ($) {
}

onFocusOut(event) {
setTimeout(() => {
// Autosubmit if the user leaves the input and the input has been manually changed.
// Only for non-instrumented mode — instrumented inputs (e.g. TermInput) handle
// autosubmit themselves via BaseInput.autoSubmit() with proper term data.
if (
! this.instrumented
&& this.hasBeenManuallyChanged
&& ! this.hasSuggestions()
&& this.shouldAutoSubmit()
) {
// Reset this flag since the user has navigated away from the input and the submit event
// will be triggered by the input's form submit event handler.
this.hasBeenManuallyChanged = false;
$(this.input.form).trigger('submit', {submittedBy: this.input});
}
}, 300);

if (this.completedInput === null) {
// If there are multiple instances of Completer bound to the same suggestion container
// all of them try to handle the event. Though, only one of them is responsible and
Expand All @@ -498,6 +522,10 @@ define(["../notjQuery"], function ($) {
}

this.hideSuggestions();

// This triggers the autosubmit if the user navigates away from the input for non-instrumented mode.
// as the input is reset to the manually changed value once suggestions are hidden.
this.hasBeenManuallyChanged = true;
Copy link
Copy Markdown
Member

@nilmerg nilmerg Mar 20, 2026

Choose a reason for hiding this comment

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

This has no effect right now, right? It would only have one, if the flag is being checked 50ms afterwards again, but that's not the case right now.

Copy link
Copy Markdown
Contributor Author

@raviks789 raviks789 Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I need this after the changes I have pushed. I have moved the logic inside the timer. So after the suggestions are hidden and the input is changed back to the manually changed input, this flag will be set and after which the autosubmit will be triggered.

Copy link
Copy Markdown
Member

@nilmerg nilmerg Mar 20, 2026

Choose a reason for hiding this comment

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

Aye, but shouldn't this then only set to true in case suggest is used to populate completedValue again?

}
}, 250);
}
Expand Down Expand Up @@ -631,6 +659,11 @@ define(["../notjQuery"], function ($) {

onKeyDown(event) {
let suggestions;
const keys = ['Tab', 'ArrowDown', 'ArrowUp'];
if (keys.includes(event.key) && (event.target === this.input && this.hasSuggestions())) {
Copy link
Copy Markdown
Member

@nilmerg nilmerg Mar 20, 2026

Choose a reason for hiding this comment

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

event.target === this.input confused me at first, maybe it's better to check for ! this.instrumented?

Copy link
Copy Markdown
Contributor Author

@raviks789 raviks789 Mar 23, 2026

Choose a reason for hiding this comment

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

event.target === this.input confused me at first,

the reason I have used event.target === this.input is because, while in the input, if any of the keys above this condition has been pressed, the user navigates to the suggestions. And this has to set the hasBeenManuallyChanged flag to false.

maybe it's better to check for ! this.instrumented?

Do you mean additionally to the condition I have used or instead of event.target === this.input?

I think we could discuss this offline.

// Disable the autosubmit if the user navigates away from the input but is within the suggestions.
this.hasBeenManuallyChanged = false;
}

switch (event.key) {
case ' ':
Expand Down Expand Up @@ -676,7 +709,10 @@ define(["../notjQuery"], function ($) {
break;
case 'Escape':
if (this.hasSuggestions()) {
this.hideSuggestions()
this.hideSuggestions();
// This triggers the autosubmit if the user navigates away from the input for non-instrumented mode.
// as the input has the manually changed value.
this.hasBeenManuallyChanged = true;
Copy link
Copy Markdown
Member

@nilmerg nilmerg Mar 20, 2026

Choose a reason for hiding this comment

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

Please add a comment stating that this also triggers an auto-submit in case the user didn't change anything. (user types, suggestions pop up, user closes them, but also removes what was typed)

This may not be a problem right now, but for the future this should be documented to be able to quickly find it.

Copy link
Copy Markdown
Contributor Author

@raviks789 raviks789 Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

If the user closes the suggestions and then removes what was typed, the suggestions will pop up once again and the user has to close the suggestions again.

What I want to mention with this is, is it not similar to changing the output manually again?

Copy link
Copy Markdown
Member

@nilmerg nilmerg Mar 20, 2026

Choose a reason for hiding this comment

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

Actually, I see no reason for this anymore at all. You're tracking the user's interaction so detailed now, does pushing escape still revert some interaction that set the flag to false?

Copy link
Copy Markdown
Contributor Author

@raviks789 raviks789 Mar 23, 2026

Choose a reason for hiding this comment

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

The hasBeenManuallyChanged flag will be set to false, once the focus is in the suggestions and this might not trigger autosubmit when the Escape is pressed, and the user clicks somewhere else in the form.

Now that I am explaining this, I think, this.hasBeenManuallyChanged = true; should happen in onSuggestionKeyDown event, where the selection is cleared and the manually changed value is populated in the input.

event.preventDefault();
}

Expand Down Expand Up @@ -712,7 +748,6 @@ define(["../notjQuery"], function ($) {

onInput(event) {
let input = event.target;

if (input.minLength > 0 && input.value.length < input.minLength) {
return;
}
Expand All @@ -730,6 +765,9 @@ define(["../notjQuery"], function ($) {
dataElement.value = input.value;
}

// This flag triggers the autosubmit if the user navigates away from the input for non-instrumented mode
// and the input has the manually changed value.
this.hasBeenManuallyChanged = true;
let [value, data] = this.prepareCompletionData(input);
this.completedInput = input;
this.completedValue = value;
Expand Down
Loading