[Bug]: Checkbox with size AND tabindex=-1 exceeds callstack

[Westbrook]

Code of conduct

• [X] I agree to follow this project's code of conduct.

Impacted component(s)

Checkbox, likely others

Expected behavior

<sp-checkbox size="s" tabindex="-1">Chekbox</sp-checkbox> should just work.

Actual behavior

Causes am exceeds call stack error.

Screenshots

No response

What browsers are you seeing the problem in?

No response

How can we reproduce this issue?

https://studio.webcomponents.dev/edit/jeIGAXHMUrTp6hGMquoD/src/index.ts?branch=tab-index%40wfVpXdsTYhf9mY2slxW4t2Y2SjC3&p=stories

Sample code that illustrates the problem

No response

Logs taken while reproducing problem

No response

[bangank36]

@Westbrook I can reproduce the issue on the lastest version, the setter 'tabindex()' caused this stack error but I not quite sure what the class Focusable do, do you mind to give some background info tips, thanks https://github.com/adobe/spectrum-web-components/blob/729adb1b84bcb1f66bfa21229784b89454ffd7c4/tools/shared/src/focusable.ts#L77

I was looking at this issue mentioned about the tabIndex=-1 issue happens, but there is no clue yet... https://github.com/flatpickr/flatpickr/issues/1730

[bangank36]

This line on the tabIndex setter will re-trigger the size setter, making the loop happen, my suggestion is skip edit the tabIndex once we know it is equal -1, eg: https://github.com/adobe/spectrum-web-components/blob/729adb1b84bcb1f66bfa21229784b89454ffd7c4/tools/shared/src/focusable.ts#L107

The condition is already tabIndex === -1 || this.disabled, I think we can get rid of the line above, what do you think? I removed it without issue, but maybe I missed something else

nit: Found a typo cange in the comment https://github.com/adobe/spectrum-web-components/blob/729adb1b84bcb1f66bfa21229784b89454ffd7c4/tools/shared/src/focusable.ts#L105

[Westbrook]

Have you been able to confirm a why the line you've pointed to triggers the size setter? It doesn't seem like it should, being it doesn't actually set size. Until we better understand the cause here, we shouldn't look to change any of the underlying code as it could have side effects in other contexts.

[bangank36]

Let me try to explain what I found... First of all, the CheckboxBase inherited Focusable and Checkbox inherited Size, that explains the connection between size and tabIndex attributes that cause this issue

Next, method attributeChangedCallback() of @lit/reactive-element ( which a dependency of this repo ) will be fired whenever we use setAttribute on the element

That will be the case which causes the issue

• On render the tabindex attribute is mapped to property on set tabIndex(tabIndex: number)
• Since we manually set the tabIndex attribute using setAttribute it will re-run the attributeChangedCallback()
• Same happens on size
• I think it making an infinite loop that exceed the callstack

[bangank36]

@Westbrook any thoughts on this? Thx

[Westbrook]

Recent changes to Checkbox seem to remove this situation from the repro link.