-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
c6f0206
ee71399
a195165
18750a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -268,6 +269,7 @@ define(["../notjQuery"], function ($) { | |
|
|
||
| complete(input, value, data) { | ||
| $(input).focus({ scripted: true }); | ||
| this.hasBeenManuallyChanged = false; | ||
|
|
||
| if (this.instrumented) { | ||
| if (! Object.keys(data).length) { | ||
|
|
@@ -476,6 +478,17 @@ define(["../notjQuery"], function ($) { | |
| } | ||
|
|
||
| onFocusOut(event) { | ||
| // 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.shouldAutoSubmit()) { | ||
| this.hasBeenManuallyChanged = false; | ||
| let input = event.target; | ||
| setTimeout(() => { | ||
| $(input.form).trigger('submit', { submittedBy: 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 | ||
|
|
@@ -498,6 +511,7 @@ define(["../notjQuery"], function ($) { | |
| } | ||
|
|
||
| this.hideSuggestions(); | ||
| this.hasBeenManuallyChanged = true; | ||
| } | ||
| }, 250); | ||
| } | ||
|
|
@@ -655,6 +669,7 @@ define(["../notjQuery"], function ($) { | |
| break; | ||
| case 'Tab': | ||
| suggestions = this.termSuggestions.querySelectorAll('[type="button"]'); | ||
| this.hasBeenManuallyChanged = false; | ||
nilmerg marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (suggestions.length === 1) { | ||
| event.preventDefault(); | ||
| let input = event.target; | ||
|
|
@@ -676,13 +691,15 @@ define(["../notjQuery"], function ($) { | |
| break; | ||
| case 'Escape': | ||
| if (this.hasSuggestions()) { | ||
| this.hasBeenManuallyChanged = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Now that I am explaining this, I think, |
||
| this.hideSuggestions() | ||
| event.preventDefault(); | ||
| } | ||
|
|
||
| break; | ||
| case 'ArrowUp': | ||
| suggestions = this.termSuggestions.querySelectorAll('[type="button"]'); | ||
| this.hasBeenManuallyChanged = false; | ||
| if (suggestions.length) { | ||
| event.preventDefault(); | ||
| this.moveToSuggestion(true); | ||
|
|
@@ -691,6 +708,7 @@ define(["../notjQuery"], function ($) { | |
| break; | ||
| case 'ArrowDown': | ||
| suggestions = this.termSuggestions.querySelectorAll('[type="button"]'); | ||
| this.hasBeenManuallyChanged = false; | ||
| if (suggestions.length) { | ||
| event.preventDefault(); | ||
| this.moveToSuggestion(); | ||
|
|
@@ -712,6 +730,7 @@ define(["../notjQuery"], function ($) { | |
|
|
||
| onInput(event) { | ||
| let input = event.target; | ||
| this.hasBeenManuallyChanged = true; | ||
|
|
||
| if (input.minLength > 0 && input.value.length < input.minLength) { | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
suggestis used to populatecompletedValueagain?