Adaptivecards: [Javascript][Extensibility] [Rendering breaks on registering custom element for multiple/expanded choiceset]

[aroraketan]

Platform

What platform is your issue or question related to? (Delete other platforms).

• [ ] JavaScript

Author or host

Host: Microsoft Teams (Multi-window).

Version of SDK

JS v2.5.0.

Details

On registering a custom element for ChoiceSet , the rendering started breaking with following error:

On digging deeper, I found that this._toggleInputs was undefined in case of custom elements.

I tried to compare the usage of the above prop in older versions as it was working fine, I found that in older versions there were checks on it being undefined even before it was used in loops. This check was missing in the new function updateInputControlAriaControlledBy.
One such older instance of its usage is

I, for now, have fixed by defining this._toggleInputs = []; in my custom element's implementation but I believe this should be resolved at SDK level. or let me know if we are using it wrong.

[msftbot[bot]]

please review this issue for target Milestone, Inconsistencies & Priority upon triage.

[msftbot[bot]]

Hi @aroraketan. We have acknowledged this issue report. Please continue to follow this issue for updates/progress/questions.

[shalinijoshi19]

@dclaux are you able to help take a look ? @golddove who;d been investigating

[dclaux]

@aroraketan This isn't a bug. If your custom control extends ChoiceSetInput and you override its internalRender method which is normally responsible for initializing _toggleInputs, then you also have to override all other methods that rely on how the control is rendered. That includes protected updateInputControlAriaLabeledBy(), protected internalApplyAriaCurrent() and get value().

[aroraketan]

@dclaux the custom control class won't be able to initialize _toggleInputs as it is private to ChoiceSetInput.
My concern here is that just upgrading SDK should not break the existing rendering logic which happened in this case.
With previous instances of its usage, SDK had safe checks as mentioned in above description. I feel this check should always be made before using this property. The developer implementing a custom input has no knowledge of this field and can easily skip initializing it even if it was possible!

Also, can you point me to list of methods in the docs that need to be overridden while creating a custom element?
Shouldn't SDK enforce override of all these methods in custom controls via TS?

[dclaux]

@aroraketan that's not the point - the default implementation creates toggle inputs and stores them into a private variable, then uses that variable later for aria-related logic. Your code replaces the default rendering. You are likely creating your own toggle inputs or something like that. The aria-related logic therefore also needs to be overridden so it applies to the elements your rendering logic creates.

We have made a number of accessibility-related improvement since the 2.0 release, and we've also introduced input validation. You get most of it for free, but that comes at the cost of having to override a couple more functions.

[dclaux]

And by the way you actually don't have to override protected internalUpdateAriaCurrent().

[dclaux]

Since I was fixing something else, I decided to "fix" this as well, see this PR: https://github.com/microsoft/AdaptiveCards/pull/5094

By "fix" however the only thing I'm referring to is that your code won't break anymore. However, you still have to override additional methods in order for your custom ChoiceSetInput to function properly (e.g. participate in the input validation sequence and comply with accessibility requirements).

I will update our documentation to be specific about which methods a custom input should override, but in the meantime, here is what they are:

| Method | Description |
| --------- | ------------- |
| protected updateInputControlAriaLabelledBy() | This method is called during the input validation sequence. When an input fails validation, its associated error message is displayed and the underlying input control's aria-labelledby attribute needs to be updated in order for the input to comply with accessibility requirements. Custom inputs SHOULD override updateInputControlAriaLabelledBy to apply the appropriate aria-labelledby attribute to the underlying input control. The getAllLabelIds(): string[] method can be used to retrieve the Ids of all the labels that should be specified in the aria-labelledby attribute. |
| protected internalRender(): HTMLElement | Just like for any other Adaptive Card custom element, internalRender MUST be overridden to render your input as desired. This is where you'll want to create the actual underlying input control. |
| protected get isNullable(): boolean | Indicates whether the input supports undefined values (for instance, Input.Text does, whereas Input.Toggle doesn't.) Custom inputs SHOULD override this property getter to indicate if they support undefined values. The base implementation returns true. |
| public focus() | When validation errors are encountered, the focus is automatically placed on the first input that failed validation. The base implementation of focus will in some cases just work for custom inputs. When that isn't the case, custom input controls MUST override focus to explicitly put the focus on the underlying input control. |
| public abstract isSet(): boolean | Indicates whether the input's value has been set by the user. This method is called during the input validation sequence to determine if the input passes or fails validation. Custom inputs MUST override isSet in order to participate in the input validation mechanism. |
| public isValid(): boolean | Indicates whether the value of the input is valid. This method is called during the input validation sequence to determine if the input passes or fails validation. Custom inputs SHOULD override isValid in order to participate in the input validation mechanism. The base implementation returns true. |
| public abstract get value(): any | Returns the value of the input. Custom inputs MUST override this so that the value of the input is retrieved from the underlying input control. |

[msftbot[bot]]

:tada:[email protected] has been released which fixes this issue.:tada:

Handy links:

• Release Notes
• AdaptiveCards Website
• Official NPM Designer Release
• Official NPM Templating Release
• Official NPM Renderer Release
• Official Nuget Templating Release
• Official Nuget Renderer Release
• Official Maven Renderer Release
• Official CocoaPod Renderer Release