IsCheckbox based on role and not only for input DOM elements

[vertonghenb]

To use Fluent or Fast WebComponents which doesn't use an input element as a checkbox this is needed. A checkbox can now also be based on a role attribute and not only on a input element with the type='checkbox'.

The fast-switch component has the same issue, but it's possible to override the role='checkbox' to also make this possible.
e.g.
<fast-switch role='checkbox'>Switch</fast-switch>

Fixes #24916

[SteveSandersonMS]

This looks great to me. Thanks @vertonghenb! We should be able to merge this to master shortly and hence it will be included in the earliest .NET 6 previews.

[vertonghenb]

@SteveSandersonMS I know we're in the final stages of 5.0 but is it still possible to get this in 5.0?
The specific ones:

• Blazor.Fast Checkbox
• Blazor.Fast Switch

[SteveSandersonMS]

I know we're in the final stages of 5.0 but is it still possible to get this in 5.0?

Unfortunately not. 5.0 is locked now apart from any massive issues like security fixes. We definitely wouldn't get approval to reset the whole 5.0 rollout process for adding support for a new scenario.

[vertonghenb]

I know we're in the final stages of 5.0 but is it still possible to get this in 5.0?

Unfortunately not. 5.0 is locked now apart from any massive issues like security fixes. We definitely wouldn't get approval to reset the whole 5.0 rollout process for adding support for a new scenario.

Perfectly understandable, thanks for considering!

[SteveSandersonMS]

@vertonghenb I was in the process of getting this ready to merge when I noticed it looks like there's slightly more involved in making this really work with fast-checkbox. The change in this PR is sufficient to make clicking on the fast-checkbox trigger the right events in .NET (to update a bound boolean), but it's not enough for changes on the bound boolean to cause the fast-checkbox to change its visual state.

I think what's needed is:

1. In addition to your existing change in EventForDotNet.ts, there must also be a change in BrowserRenderer.ts so the tryApplyCheckedProperty function also respects the role="checkbox" attribute
2. We need E2E tests showing this works. At first I thought we could do this without actually referencing the fast-components library, but having tried it, we'd have to replicate most of what they do for checkboxes for the test to make sense. Since we have a bigger goal of ensuring great compatibility with everything in fast-components, I think it would be better to create a dedicated E2E test page that's all about fast-components, actually references that library, and includes examples of @bind and events on as many of those components as we can.
   • As part of this, we need to decide whether the BindAttributes class we add for fast-components is just part of the E2E test project, or whether we actually ship some new Microsoft.AspNetCore.Components.Fast that contains it. Doing the latter there would be a bigger product decision, so probably as a starting point, the fast-specific BindAttributes class should just be in the E2E project. cc @danroth27 for consideration of whether we want to ship a fast-specific package in 6.0.

I know this complicates what you're proposing quite a bit, but we can't really take a change that doesn't have corresponding E2E tests otherwise we lose track of what the product actually does and can't detect if we regress it. And adding a meaningful E2E test pretty much relies on actually referencing fast-components here, or replicating enough of it that you can have a functioning checkbox without type="checkbox".

Are you still interested in implementing the rest of this? If so that's great. If not, that's totally fine too and we'd put it on the backlog for .NET 6, since having solid integration with fast-components is absolutely one of our goals there.

[vertonghenb]

@SteveSandersonMS
Wauw, I wasn't expecting a change as big as this! But the following is quite comforting.

for .NET 6, since having solid integration with fast-components is absolutely one of our goals there.

I created an integration not so long ago, wrapping the components the only 3 which are having issues so far is the Checkbox and Switch, Anchor (something to do with the href tag, that refreshes the entire page).

In addition to your existing change in EventForDotNet.ts, there must also be a change in BrowserRenderer.ts so the tryApplyCheckedProperty function also respects the role="checkbox" attribute

I'll check what I can do there, some help might needed.

We need E2E tests showing this works.

Actually... the E2E page is already here, we can move it if you like.

GitHub: https://github.com/Append-IT/Blazor.Fast
E2E: https://blazorfast.append.be/

[SteveSandersonMS]

There's already a E2E test setup for Blazor in this repo, so we'd need the new E2E tests to conform to that pattern. I think we'd be trying to show that the underlying web components (such as <fast-checkbox>) can be used directly with Blazor's @bind and onevent syntaxes without needed any extra component wrappers.

[CuddleBunny]

While supporting FAST specifically is a good place to start I think that Blazor should aim to support most web component implementations. I think using role is an acceptable constraint to force on component developers in this situation though.

[ghost]

Hi @vertonghenb.
It seems you haven't touched this PR for the last two weeks. To avoid accumulating old PRs, we're marking it as stale. As a result, it will be closed if no further activity occurs within 4 days of this comment. You can learn more about our Issue Management Policies here.