[Switch] Implement the component-per-node API

[michaldudak]

Closes #4

[michaldudak]

ClassNameConfigurator will be removed, so it doesn't make sense to update its tests

[colmtuite]

Tested it locally. All looks good except the inline <input> styles. Currently, it's not removed from document flow and is taking up screen space. We also need to block pointer events.

Suggested inline CSS:

height: 1px;
margin: 0;
opacity: 0;
overflow: hidden;
pointer-events: none;
transform: translateX(-100%);
width: 1px;

Edit: James is handling this differently in his Checkbox PR, so I guess let's just go with that CSS.

    border: 0px;
    clip: rect(0px, 0px, 0px, 0px);
    height: 1px;
    margin: -1px;
    overflow: hidden;
    padding: 0px;
    position: fixed;
    white-space: nowrap;
    width: 1px;
    top: 0px;
    left: 0px;

[michaldudak]

All right, let's extract these styles to a reusable function as they will be used in many places.

EDIT:
Actually, we already have an utility for this in @mui/utils. It's slightly different, though. @atomiks, @colmtuite, do you think this will work as well?

{
  border: 0,
  clip: 'rect(0 0 0 0)',
  height: '1px',
  margin: -1,
  overflow: 'hidden',
  padding: 0,
  position: 'absolute',
  whiteSpace: 'nowrap',
  width: '1px',
}

[colmtuite]

Yep that looks good @michaldudak. margin value should be -1px though.

I assume you'll copy this across to the base ui repo?

[michaldudak]

I'll fix it in the utils, then. I don't plan to copy it - we depend on @mui/utils anyway, so we can just import it.

[atomiks]

The prop getters here should likely be memoized, so they can be passed to a memo'd component via the render prop where necessary. I also think it's cleaner to inline the event handlers inside the props themselves?

[michaldudak]

Done. As for inlining - I'm not entirely convinced. I don't like too many levels of indentation. I moved the event handlers closer to these functions so it's easier to see both at the same time

[atomiks]

I'm mostly fine with either, but I do think inlining is simpler and the better option:

• You don't need to specify types for the parameters, and there's less indirection
• It's easier to read compared to the double function
• It's colocated with the other props
• The indentation is the same (8 spaces):

Separate

  const createHandleButtonClick = React.useCallback(
    (otherProps: React.ButtonHTMLAttributes<HTMLButtonElement>) =>
      (event: React.MouseEvent<HTMLButtonElement>) => {
        inputRef.current?.click();
12345678^
      },
    [readOnly],
  );

  const getButtonProps: UseSwitchReturnValue['getButtonProps'] = React.useCallback(
    (otherProps = {}) => ({
      onClick: createHandleButtonClick(otherProps),
    }),
    [checked, disabled, readOnly, createHandleButtonClick],
  );

Inline

  const getButtonProps: UseSwitchReturnValue['getButtonProps'] = React.useCallback(
    (otherProps = {}) => ({
      onClick(event) {
        inputRef.current?.click();
12345678^
      },
    }),
    [checked, disabled, readOnly],
  );

Overall, I'm not that concerned and think either is fine, but I feel inlining is easier to read and write.

[michaldudak]

@colmtuite, @atomiks, I think this is ready for the final review.

[atomiks]

The API docs show import { SwitchThumb } from '@mui/base/SwitchThumb' being possible. Should this be the case, or just have the generator updated to show Switch.Thumb usage?

[michaldudak]

It's an issue with the API docs generator I haven't fixed yet, but we can do this in a separate PR.

[colmtuite]

Being able to focus a disabled Switch is bothering me, but the same goes for all disabled components. We can chat about this elsewhere, just wanted to mention it here in case we loop back around later.