Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.09%. Comparing base (28bc599) to head (4448a5e).
Report is 55 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #8672      +/-   ##
==========================================
+ Coverage   89.82%   90.09%   +0.26%     
==========================================
  Files         412      418       +6     
  Lines       11878    12055     +177     
  Branches     2364     2365       +1     
==========================================
+ Hits        10670    10861     +191     
+ Misses        681      660      -21     
- Partials      527      534       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ScarletKuro @henon for review, not sure whether this can be considered a breaking change since this messes with input ids (and affected inputs no longer use FieldId from MudComponentBase)

Potential next steps:

I saw the FieldId property on MudComponentBase but out of hundreds of inheritors of MudComponentBase the components affected by this PR and MudField + MudDateRangePicker are the only ones that use(d) it.

MudField is not necessarily a form control. According to the documentation it can be used for static text (in this case it must not have a label tag) or it can be used to create custom form controls (in this case it must have a label tag). The API should most likely allow the user to decide whether a label tag or some other text container should be used for the Label property.

Currently it always has a label tag, which triggers accessibility issues:

MudDateRangePicker is a bit of a special case, it has two inputs (one for start date, one for end date). It should therefore probably have two labels (they can probably be hidden visually but still visible for a screen reader, that seems to be the right approach). The currently visible label might need to be replaced with a different text container.

Couldn't you have added FieldId as the id for the <input>, given that it was already used for the ForId. Why add CalculatedFieldId and the additional InputId parameter.

I suppose the InputId makes adding an id slightly more discoverable?

Couldn't you have added FieldId as the id for the <input>, given that it was already used for the ForId. Why add CalculatedFieldId and the additional InputId parameter.

I suppose the InputId makes adding an id slightly more discoverable?

I think FieldId should be removed from MudComponentBase since only a limited subset of inheritors actually need it, basically only the components affected by this PR and MudField + MudDateRangePicker (both of which need their own adjustments for label/id accessibility issues).

The InputId is an extra, though yeah it seems more user friendly than <MudTextField id="my-custom-id" .../>

Edit: because you have to know the inner workings of the components to know that id will be applied to the input tag

Breaking changes are ok; If you think FieldId can be removed in a beneficial way it should be explored.

Don't have an opinion on that, as I don't know much accessibility flow, can only link a PR that added it #4107 and that i was also for the WCAG

@brianseim If you could review this change, we'd appreciate it a lot.

This is marked as breaking change but I don't understand what exactly got broken. Can you summarize for the migration guide?

This is marked as breaking change but I don't understand what exactly got broken. Can you summarize for the migration guide?

I think I was worried about the userAttributes id being overwritten (or no longer using FieldId) when I first published the PR. I've just reviewed the code and I don't see any breaking change anymore. Should be safe to remove the tag (and sorry for not stating the reason for marking it as breaking change in the first place!).

Thanks