[material-ui][Checkbox] Asterisk placement aligned correctly

[axelbostrom]

Closes #39626

Demo: https://codesandbox.io/s/https-github-com-mui-material-ui-pull-39721-rdd4kc?file=/src/App.tsx

• I have followed (at least) the PR section of the contributing guide.

[mui-bot]

Netlify deploy preview

https://deploy-preview-39721--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 926377b

[asdotdev]

This fixes asterisk placement. There is no need for flex so using Stack is unnecessary.

But div can be used in place of the span as label text and the asterisk is already wrapped in span.
Because using span inside span is not good, the Stack component also uses div.

[mj12albert]

@axelbostrom I suggest just overriding the display property on the Stack instead to lessen the impact of the change (e.g. someone could be targeting classes on that Stack for styling)

diff --git a/packages/mui-material/src/FormControlLabel/FormControlLabel.js b/packages/mui-material/src/FormControlLabel/FormControlLabel.js
index fa0d4cf32c..be38944c23 100644
--- a/packages/mui-material/src/FormControlLabel/FormControlLabel.js
+++ b/packages/mui-material/src/FormControlLabel/FormControlLabel.js
@@ -166,7 +166,7 @@ const FormControlLabel = React.forwardRef(function FormControlLabel(inProps, ref
     >
       {React.cloneElement(control, controlProps)}
       {required ? (
-        <Stack direction="row" alignItems="center">
+        <Stack direction="row" alignItems="center" sx={{ display: 'block' }}>
           {label}
           <AsteriskComponent ownerState={ownerState} aria-hidden className={classes.asterisk}>
             &thinsp;{'*'}

[mj12albert]

Because using span inside span is not good

I don't think there is anything inherently wrong with spans inside spans 🤔

[axelbostrom]

@mj12albert Right, but I feel like using stack for the asterisk is very uncessesary, since there is no need for horizontal or vertical aligntment for the asterisk. So yeah either a div or span inside span. I don't think there is an issue with a span inside a span either, but not sure.

[asdotdev]

I don't think there is anything inherently wrong with spans inside spans 🤔

Yes, you are right there is nothing wrong with doing it. It's just less semantic.

The span element is a generic inline container for phrasing content, which does not inherently represent anything. It should be used only when no other semantic element is appropriate.

[mj12albert]

I feel like using stack for the asterisk is very uncessesary

You are right that is is unnecessary, but to remove it entirely now is a bigger change (that I'd like to avoid) than updating a style

[ZeeshanTamboli]

Just a quick note: Stack was introduced in #37831 to address the asterisk placement issue with different label positions in #37281. Removing it might affect #37281.

Edit: I should have added visual regression tests in that PR to explain the purpose of the Stack component. This way, the CI would fail when the Stack component is removed.

[axelbostrom]

@ZeeshanTamboli ok I will look into this this weekend or on monday.

Anyway, what is the best solution using span stack or div?

[mj12albert]

what is the best solution using span stack or div

Let's keep using the Stack and try to fix the issue by adjusting the styles alone to avoid breaking changes

[axelbostrom]

This change ensures that the asterisk correctly appears at the end of the label text within the FormControlLabel component, even when the label spans multiple lines, but uses nested spans but keeps the stack component.

[asdotdev]

Current changes add unnecessary HTML elements.

Simply this change can work.

- <Stack direction="row" alignItems="center">
-     <span>
+ <Stack display="block">
            {label}
            <AsteriskComponent ownerState={ownerState} aria-hidden className={classes.asterisk}>
                &thinsp;{'*'}
            </AsteriskComponent>
-     </span>
  </Stack>

[mj12albert]

We should go with @amitabh-sahu's suggestion, it's highly preferable we don't alter the structure to avoid breaking changes

[mj12albert]

@axelbostrom Looks good ~ thanks for spending the time to work on this ✨

[mj12albert]

BTW I've added a sandbox to the description and checked out @ZeeshanTamboli's comment:

The changes did not break different labelPlacements

[axelbostrom]

Thanks for the help! Not the most code I have written but a great learning experience :)