How about remove the Label warpper for Switch?

[PeterlitsZo]

If we want to to let the label be before of the switch, we need:

<Flex
  sx={{
    justifyContent: 'space-between',
    alignItems: 'center',
    py: 4,
  }}>
  <Label htmlFor="enable-email-alerts" sx={{ flex: 1 }}>
    Enable email alerts?
  </Label>
  <Box>
    <Switch id="enable-email-alerts" />
  </Box>
</Flex>

But I find that Switch is already a Label element. Why? I think this will be better:

<Label sx={{
    display: 'flex',
    justifyContent: 'space-between',
    alignItems: 'center',
    py: 4,
  }}>
  <span>Enable email alerts?</span>
  <Switch /> {/* A React.Fragment but not label element */}
</Label>

[PeterlitsZo]

It will break something... so we can create a new component PureSwitch. How about it?

[PeterlitsZo]

I will try in my project.

[PeterlitsZo]

By the way, I find that the Box set margin: 0, which let the parent's & > * + *'s margin-left: 5px rule not works.

Need we set the Box's margin: 0?

[PeterlitsZo]

Here is my work, it works well for me:

My code!

import React from 'react';
import { Box, Label } from 'theme-ui';
import type { CSSProperties as CSSProp } from 'theme-ui';

const GUTTER = 2;
const SIZE = 18;

interface SwitchProps {
  gutter?: CSSProp['height'] & CSSProp['width'] & CSSProp['borderRadius'];
  size?: CSSProp['height'] & CSSProp['width'] & CSSProp['borderRadius'];
  className?: string;
  label?: string;
  variant?: string;
  [key: string]: any;
}

export default React.forwardRef(
  (props: SwitchProps, ref: React.ForwardedRef<HTMLInputElement>) => {
    let {
      gutter = GUTTER,
      size = SIZE,
      className,
      label,
      variant = 'switch',
      ...otherProps
    } = props;
    if (typeof gutter === 'number') gutter = gutter + 'px';
    if (typeof size === 'number') size = size + 'px';

    // Hidden checkbox, the really element.
    const Checkbox = (
      <Box
        ref={ref}
        as="input"
        // @ts-ignore
        type="checkbox"
        __themeKey="forms"
        aria-label={label}
        {...otherProps}
        sx={{
          position: 'absolute',
          opacity: 0,
          zIndex: -1,
          width: 1,
          height: 1,
          overflow: 'hidden',
        }}
      />
    );

    // The switch just for show.
    const Switch = (
      <Box
        css={{
          padding: gutter,
        }}
        // @ts-ignore
        __themeKey="forms"
        variant={variant}
        className={className}
        __css={{
          position: 'relative',
          flexShrink: 0,
          bg: 'gray',
          borderRadius: size,
          height: `calc(${size} + ${gutter} * 2)`,
          width: `calc(${size} * 2 + ${gutter} * 2)`,
          'input:disabled ~ &': {
            opacity: 0.5,
            cursor: 'not-allowed',
          },
          '& > div': {
            display: 'flex',
            alignItems: 'center',
            borderRadius: '50%',
            height: size,
            width: size,
            bg: 'white',
            boxShadow: '0 1px 2px rgba(0, 0, 0, 0.1)',
            position: 'relative',
            transform: 'translateX(0%)',
            transition: `transform 240ms cubic-bezier(0.165, 0.840, 0.440, 1.000)`,
          },
          'input:checked ~ &': {
            bg: 'primary',
            '> div': {
              transform: 'translateX(100%)',
            },
          },
        }}
      >
        <Box />
      </Box>
    );

    if (label) {
      return (
        <Label sx={{ cursor: 'pointer' }}>
          {Checkbox}
          {Switch}
          <span>{label}</span>
        </Label>
      );
    }

    return (
      <React.Fragment>
        {Checkbox}
        {Switch}
      </React.Fragment>
    );
  },
);

[lachlanjc]

Box's margin: 0 is designed to be our CSS reset, without requiring global CSS, which we discourage. If you're setting space inside a flexbox container, I'd recommend gap instead of margin personally.

The customization here is an interesting idea! The other way to go would be MUI-like startLabel/endLabel props, then we could deprecate label but have it map to endLabel for a few releases with a warning.

[PeterlitsZo]

Box's margin: 0 is designed to be our CSS reset, without requiring global CSS, which we discourage. If you're setting space inside a flexbox container, I'd recommend gap instead of margin personally.

I do use gap instead of margin. But some browser do not support gap... If margin is OK to use, it will be better...

The customization here is an interesting idea! The other way to go would be MUI-like startLabel/endLabel props, then we could deprecate label but have it map to endLabel for a few releases with a warning.

startLabel? endLabel? yes, it is a good idea! Do you need a PR?

[PeterlitsZo]

Although I think I get it, but I cannot find startLabel/endLabel in MUI...

[lachlanjc]

They use startIcon/endIcon on Buttons. If you'd like to write a PR, we'd love that!

[PeterlitsZo]

How about this? Do you like it?

[PeterlitsZo]

By the way, do you think that we can change the inner div's color rather than white is a better idea?

By the way, @lachlanjc are you still 19?

[lachlanjc]

Personally I'd vote for no more than 2 label props—either the text string & a position one, or the start/endLabel props. It's too confusing when to use endLabel vs label. We'd keep label for a bit as a deprecated compatability shim. cc @hasparus on API preferences

I'm 20 now!

[lachlanjc]

By the way, do you think that we can change the inner div's color rather than white is a better idea?

Not sure what you mean by this

[hasparus]

cc @hasparus on API preferences

TBH I'd rather have just one label prop and labelPosition or something like that?

{
  label?: string,
  /** @default "start" */
  labelPosition?: "start" | "end",
}

[lachlanjc]

Was also thinking about this. Works for me! Also then won't be a breaking change which is preferable, & it's more consistent with other components.

[PeterlitsZo]

OK.

[PeterlitsZo]

How about this? I also update index.d.ts for type. (By the way, a stupid question -- why we do not just use ts rather than js?)

I'm 20 now!

I'm also 20!

[PeterlitsZo]

By the way, do you think that we can change the inner div's color rather than white is a better idea?

Not sure what you mean by this

I mean, the switch for show have two div: the outer div, and the inner div. The inner div is a circle, which it's color is always white. Well, now I do not think that change it to another is a good idea. Those who want to change color may use CSS rule to change it.

[PeterlitsZo]

Hello? I'd like to know could I give you PR now...

[lachlanjc]

Oh phenomenal! Both Piotr & I are unpaid volunteers maintains the projects each with full-time jobs, so we sometimes are less active for a few days when we're doing other things. Especially if we suggested an implementation, you never need permission to open a PR, & we'll just review it as soon as we can!

[PeterlitsZo]

PR: #2137

This issue is closed.