Completer.js: Ensure autosubmit is triggered if no suggestion is selected by raviks789 · Pull Request #355 · Icinga/ipl-web

raviks789 · 2026-03-04T08:12:46Z

Summary

If Completer has a auto-submit attribute in its dataset, Completer does not trigger autosubmit for manual inputs and the submit event is only triggered when a suggestion is clicked and if a classic 'class' => 'autosubmit' attribute is used, the submit event will be triggered before the suggestion click and this in turn never fills the completer input with the suggestion data.

Fix

Ensures the form submits automatically when:

The completer is not part of term input
The completer has the auto-submit dataset attribute
No suggestion is explicitly selected on manual input

Result

The solution triggers makes the completer to trigger autosubmit on manual input as expected when the focus goes away from the completer, even though no suggestion has been clicked. The classic autosubmit 'class' => 'autosubmit' would still not work with the completer.

… not instrumented Ensure the form submits automatically when: - The completer is not part of term input or instrumented - The completer has the autosubmit dataset attribute - No suggestion is explicitly selected on manual input

nilmerg

Is this still AI-generated? If not, remove the disclosure.

nilmerg · 2026-03-06T11:34:36Z

asset/js/widget/Completer.js

        constructor(input, instrumented = false) {
            this.input = input;
            this.instrumented = instrumented;
+            this.hasNotBeenCompleted = false; // Flag to identify if the input has been completed at least once.


Below: ! this.hasNotBeenCompleted

Double negation.

Suggestion: this.hasBeenManuallyChanged

nilmerg · 2026-03-06T11:39:56Z

asset/js/widget/Completer.js

@@ -492,12 +494,20 @@ define(["../notjQuery"], function ($) {
                    && ! this.termSuggestions.contains(document.activeElement)
                ) {
                    // Hide the suggestions if the user doesn't navigate them
-                    if (input !== completedInput) {
+                if (completedInput !== null && input !== completedInput) {
                        // Restore input if a suggestion lost focus


Leave this code path alone please. For one, the comment is not applicable anymore as it doesn't mention your change. Else, the remaining code wasn't intended to handle completedInput = null and you only adjusted one of three cases. Don't try to generalize code like this.

At the very start of the method:

Check if ! this.instrumented && this.hasBeenManuallyChanged && this.shouldAutoSubmit()

Then immediately reset this.hasBeenManuallyChanged

Now trigger the submit, either immediately or with its own timout

raviks789 · 2026-03-06T12:00:29Z

Is this still AI-generated? If not, remove the disclosure.

No, after these changes, it is not AI generated anymore. But don't we have to mention that it was initially AI generated and then adjusted (made necessary changes ) as I have added in the disclosure?

- rename property hasNotBeenCompleted to hasBeenManuallyChanged - Separate autosubmit logic from suggestionkiller

nilmerg

There's still an issue with duplicated auto submissions:

Type in the input and wait for suggestions
Choose a suggestion with the mouse
Notice an aborted request in the network log

The request that is aborted should have never occurred because of the interaction with the suggestions.

This gets worse if you try to navigate the suggestion list with the keyboard.

No, after these changes, it is not AI generated anymore. But don't we have to mention that it was initially AI generated and then adjusted (made necessary changes ) as I have added in the disclosure?

Once any trace of AI generated code is gone, there's no reason for the disclosure anymore. This is about for what AI was being used, nothing else. I'll write up my reasoning some time soon probably, so it's clear to everyone why.

asset/js/widget/Completer.js

nilmerg · 2026-03-20T09:48:27Z

asset/js/widget/Completer.js

                    break;
                case 'Escape':
                    if (this.hasSuggestions()) {
+                        this.hasBeenManuallyChanged = true;


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.

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?

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?

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.

asset/js/widget/Completer.js

nilmerg · 2026-03-20T09:53:45Z

asset/js/widget/Completer.js

                    }

                    this.hideSuggestions();
+                    this.hasBeenManuallyChanged = true;


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.

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.

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

raviks789 · 2026-03-20T12:51:36Z

Once any trace of AI generated code is gone, there's no reason for the disclosure anymore. This is about for what AI was being used, nothing else. I'll write up my reasoning some time soon probably, so it's clear to everyone why.

I have now removed the AI part in the PR description.

nilmerg

So, I think it works better now than ever before. But I have a major complaint left, but I'm not sure if this can be mitigated at all as it may mean questioning the entire proposed change:

I can't stand it, in case JS is interfering with how I navigate through a UI. One key aspect to this is, that focus stays where I put it. Take the SearchBar or the TermInput (i.e. those who instrument the completer). They both handle auto-submission in a way that doesn't affect the currently focused element too much, if at all. However, the completer will now while non-instrumented auto-submit on focus-out, after 300ms. By this time the focus is long somewhere else, visibly, but then reverted back. Imagine a user typing already in some other field, or maybe an AI-Agent if you can't imagine someone (me 😅 ) typing so fast. (Though, no idea if an AI is really interacting this way with Web UIs…)

Anyway, this is only going to be used in Director right now, which already has a habit of being disgustingly bad in terms of focus handling. So users of Director won't notice this that much. 🙄

nilmerg · 2026-03-20T15:06:51Z

asset/js/widget/Completer.js

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


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

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.

nilmerg · 2026-03-20T15:38:24Z

asset/js/widget/Completer.js

                    break;
                case 'Escape':
                    if (this.hasSuggestions()) {
+                        this.hasBeenManuallyChanged = true;


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?

nilmerg · 2026-03-20T15:41:16Z

asset/js/widget/Completer.js

                    }

                    this.hideSuggestions();
+                    this.hasBeenManuallyChanged = true;


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

cla-bot bot added the cla/signed label Mar 4, 2026

raviks789 requested review from lippserd and nilmerg March 4, 2026 08:25

raviks789 force-pushed the fix/completer-autosubmit branch from 9268541 to 6114283 Compare March 4, 2026 09:05

raviks789 self-assigned this Mar 5, 2026

raviks789 force-pushed the fix/completer-autosubmit branch 2 times, most recently from 2665ab0 to 02f6d4a Compare March 5, 2026 09:48

raviks789 force-pushed the fix/completer-autosubmit branch from 9597105 to c6f0206 Compare March 6, 2026 09:00

nilmerg requested changes Mar 6, 2026

View reviewed changes

fix: resolve review comments

ee71399

- rename property hasNotBeenCompleted to hasBeenManuallyChanged - Separate autosubmit logic from suggestionkiller

raviks789 force-pushed the fix/completer-autosubmit branch from 678f47b to ee71399 Compare March 6, 2026 12:09

raviks789 requested a review from nilmerg March 6, 2026 12:10

nilmerg requested changes Mar 6, 2026

View reviewed changes

raviks789 force-pushed the fix/completer-autosubmit branch from ec8d5bf to 31bf2fb Compare March 10, 2026 14:10

raviks789 requested a review from nilmerg March 10, 2026 14:10

raviks789 mentioned this pull request Mar 11, 2026

New custom property support Icinga/icingaweb2-module-director#2972

Draft

raviks789 force-pushed the fix/completer-autosubmit branch from 31bf2fb to ccbab73 Compare March 11, 2026 15:39

fix: autosubmit only if the suggestion is closed

a195165

raviks789 force-pushed the fix/completer-autosubmit branch from ccbab73 to a195165 Compare March 11, 2026 15:57

nilmerg requested changes Mar 20, 2026

View reviewed changes

raviks789 force-pushed the fix/completer-autosubmit branch from 9428639 to 54cdfa6 Compare March 20, 2026 12:40

raviks789 requested a review from nilmerg March 20, 2026 12:47

Completer: Move autosubmit logic into the timer onFocusOut

18750a2

raviks789 force-pushed the fix/completer-autosubmit branch from 54cdfa6 to 18750a2 Compare March 20, 2026 12:49

nilmerg requested changes Mar 20, 2026

View reviewed changes

Conversation

raviks789 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Result

Uh oh!

nilmerg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raviks789 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nilmerg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raviks789 Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raviks789 Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raviks789 commented Mar 20, 2026

Uh oh!

nilmerg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

raviks789 commented Mar 4, 2026 •

edited

Loading

raviks789 commented Mar 6, 2026 •

edited

Loading

raviks789 Mar 20, 2026 •

edited

Loading

raviks789 Mar 20, 2026 •

edited

Loading