Skip to content

Introduce AutosubmitIndicationDecorator#356

Open
sukhwinder33445 wants to merge 1 commit intomainfrom
feature/add-autosubmit-indicator
Open

Introduce AutosubmitIndicationDecorator#356
sukhwinder33445 wants to merge 1 commit intomainfrom
feature/add-autosubmit-indicator

Conversation

@sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Mar 5, 2026

This PR introduces AutosubmitIndicationDecorator, a decorator that adds an indicator to elements with autosubmit behavior. The current implementation supports checkboxes only.

Before:

Screenshot 2026-03-18 at 16 13 20

After:

Before clicking the checkbox:
Screenshot 2026-03-05 at 15 24 16
Processing after click:
Screenshot 2026-03-05 at 15 24 48

resolves #344
requires Icinga/ipl-html#188

The test class is written using AI.

@sukhwinder33445 sukhwinder33445 self-assigned this Mar 5, 2026
@cla-bot cla-bot bot added the cla/signed label Mar 5, 2026
@sukhwinder33445 sukhwinder33445 force-pushed the feature/add-autosubmit-indicator branch 3 times, most recently from d0bf89b to 62e7e35 Compare March 5, 2026 15:14
@sukhwinder33445
Copy link
Contributor Author

I’m not sure about the decorator name. Maybe AutosubmitSpinner or AutosubmitIndicator would fit better.

This comment was marked as resolved.

@sukhwinder33445
Copy link
Contributor Author

TBD: Should we define standalone CSS for the spinner animation? Currently, this depends on IcingaWeb2.

/**
* Decorates the autosubmit elements with a spinner icon
*/
class AutosubmitDecorator implements FormElementDecoration, DecoratorOptionsInterface
Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure about the decorator name. Maybe AutosubmitSpinner or AutosubmitIndicator would fit better.

I'd rather go with AutosubmitIndicationDecorator.

);
}

public function testDecoratorGeneratesIdWhenNoneIsSet(): void
Copy link
Member

Choose a reason for hiding this comment

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

This name states something else than what's tested. Or rather, what's tested isn't what this states? (aria-describedby is set irrelevant whether a custom id is passed or not.)

tl;dr, this test should cover that aria-describedby is set regardless of how the id is set. In this case when none is set, as the others cover the other cases. So the test is fine, but the name of it not… (tl;dr's are usually shorter… I suppose that's due to this being AI generated -.- (THE TEST! not my comment))

Copy link
Contributor Author

@sukhwinder33445 sukhwinder33445 Mar 17, 2026

Choose a reason for hiding this comment

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

I have renamed the tests and removed one redundant test (that checked whether an ID was generated if none was specified).

@sukhwinder33445 sukhwinder33445 force-pushed the feature/add-autosubmit-indicator branch from 62e7e35 to e261042 Compare March 17, 2026 11:24
@sukhwinder33445 sukhwinder33445 changed the title Introduce AutosubmitDecorator Introduce AutosubmitIndicationDecorator Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add autosubmit indicator to toggle sliders

3 participants