Feature/interface signaller UI

[lukasrad02]

This PR adds a new modal that allows interface signallers to interact with the software

PR Checklist

Please make sure to fulfil the following conditions before marking this PR ready for review:

• If this PR adds or changes features or fixes bugs, this has been added to the changelog
• If this PR adds new actions or other ways to alter the state, test scenarios have been added.
• I have the right to license the code submitted with this Pull Request under the mentioned license in the file LICENSE-README.md (i.e., this is my
  own code or code licensed under a license compatible to AGPL v3.0 or later, for exceptions look into LICENSE-README.md) and
  hereby license the code in this Pull Request under it.
  I certify that by signing off my commits (see In case of using third party code, I have given appropriate credit.
  We are using DCO for that, see here for more information.
• If I have used third party code and I mentioned it in the code, I also updated the inspired-by-or-copied-from-list.html list to include the links.

[ClFeSc]

As discussed, my review as far as I've gotten today.

[ClFeSc]

Maybe use this instead:

Suggested change

	// ESLint seems to be wrong here. Without the type assertion, we receive an ExerciseSimulationBehaviorState
	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
	const reportBehavior = selectStateSnapshot(
	createSelectBehaviorState(
	this.simulatedRegionId,
	this.reportBehaviorId
	),
	this.store
	) as ReportBehaviorState;
	const reportBehavior = selectStateSnapshot(
	createSelectBehaviorState<ReportBehaviorState>(
	this.simulatedRegionId,
	this.reportBehaviorId
	),
	this.store
	);

This should in theory be done to all calls to createSelectBehaviorState as its B generic parameter can never be inferred from usage. But this should not be something for this PR, I think.

By the way, createSelectBehaviorState has a weird contract: It allows you to pass in any B extends ExerciseSimulationBehaviorState but filters for a condition unrelated (at the surface at least) to B, simply assuming the resulting element is a B.

[lukasrad02]

Thanks for the suggested fix! I've added it in 255fcd5

I understand why you think the contract is weird, but I claim that it makes sense given how I expect the selector to be used: If I got a UUID and know from the context what type of behavior to expect, I wouldn't check the type again, but just assume it (i.e., perform a type cast). So I'd rather keep this shorthand way of doing it

[ClFeSc]

Consider opening an issue for this and link it here.

[lukasrad02]

Moved to #1093

[ClFeSc]

These are also shown when there are no elements to select or toggle, e.g. in the sent alarm groups dialog. Maybe there is a way to check whether there is anything meaningful to tab through.

[lukasrad02]

Nice suggestion, implemented in 9b8e6cd

[ClFeSc]

Please document which unit is expected here, e.g. by renaming the parameter to newIntervalMinutes.

[lukasrad02]

Added in 24d684f

[ClFeSc]

TODO for this PR?

[ClFeSc]

It's a little weird that these requiredBehaviors are defined twice: Once here, and again in the *ngIf of the respective ng-template.

[lukasrad02]

You're right, but I don't know how to resolve this. In the HTML template, I need the *ngIf to resolve the observable to a specific value to pass it as input into the component and in the TypeScript code, I need the array of required behaviors to en-/disable the button and hotkey to show some user feedback before the user actually clicked on the button.

What I could do:

• Change *ngIf to *appLet so that it does not look like a condition (however, it basically does the same in the end)
• Remove the templates showing an error message for the else branch, as it should never be visible due to the checks in the code behind.

But the general issue of duplication remains. Even if I only pass the ID of the region into the subcomponents, the required behaviors would still be checked twice (once in the current commands component and once in each of the subcomponents).

Do you have a suggestion how to handle this?

[ClFeSc]

I feel like some kind of unification would be good here. Currently, the modal type is declared twice, once in the secondary hotkey and once in the function called by the hotkey. Also, these calls are more or less identical between the different interactions. However, I'm also not sure how this could be unified in a reasonable way.

[lukasrad02]

I think the only way to simplify/deduplicate this would be to define an additional, reduced data model and then build the full data model from this definition. However, this feels like overall adding more overhead and complexity due to the additional indirection…

[ClFeSc]

TODO for this PR? Also for the two TODOs below.

[ClFeSc]

This is a little weird in the sense that you cannot filter by multiple phrases by entering multiple words. Maybe you could split the lowerFilter in whitespace and then require that all parts match, each using the existing filter logic.

[lukasrad02]

Nice idea, added in 9b6fa4d