Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FormControl.Label causing Layout Shift #2051

Closed
filipedeschamps opened this issue May 1, 2022 · 3 comments
Closed

FormControl.Label causing Layout Shift #2051

filipedeschamps opened this issue May 1, 2022 · 3 comments

Comments

@filipedeschamps
Copy link

Describe the bug
FormControl.Label is causing Layout Shift on page load (see gif below)

To Reproduce

  1. Create a page using FormControl with a FormControl.Label component. For example:
        <FormControl>
          <FormControl.Label>Username</FormControl.Label>
          <TextInput />
        </FormControl>
  1. Refresh the page.

Expected behavior
<FormControl.Label> should not flash on page load.

Screenshots

Gif of the page being refreshed

layout-shift

Static image showing the exact frame where <FormControl.Label> is invisible

image

Desktop (please complete the following information):

  • OS: macOS
  • Browser tested on Chrome (latest) and Safari (latest)

Additional context

There's no logic applied to the <FormControl.Label> component. Here's the code from the gif above:

return (
    <form style={{ width: '100%' }} onSubmit={handleSubmit}>
      <Box sx={{ display: 'flex', flexDirection: 'column', gap: 3 }}>
        {globalErrorMessage && <Flash variant="danger">{globalErrorMessage}</Flash>}

        <FormControl id="username">
          <FormControl.Label>Nome de usuário</FormControl.Label>
          <TextInput
            ref={usernameRef}
            onChange={clearErrors}
            name="username"
            size="large"
            autoCorrect="off"
            autoCapitalize="off"
            spellCheck={false}
            aria-label="Seu nome de usuário"
          />
          {errorObject?.key === 'username' && (
            <FormControl.Validation variant="error">{errorObject.message}</FormControl.Validation>
          )}

          {errorObject?.type === 'string.alphanum' && (
            <FormControl.Caption>Dica: use somente letras e números, por exemplo: nomeSobrenome4 </FormControl.Caption>
          )}
        </FormControl>
        <FormControl id="email">
          <FormControl.Label>Email</FormControl.Label>
          <TextInput
            ref={emailRef}
            onChange={clearErrors}
            name="email"
            size="large"
            autoCorrect="off"
            autoCapitalize="off"
            spellCheck={false}
            aria-label="Seu email"
          />
          {errorObject?.key === 'email' && (
            <FormControl.Validation variant="error">{errorObject.message}</FormControl.Validation>
          )}
        </FormControl>
        <FormControl id="password">
          <FormControl.Label>Senha</FormControl.Label>
          <TextInput
            ref={passwordRef}
            onChange={clearErrors}
            name="password"
            type="password"
            autoCorrect="off"
            autoCapitalize="off"
            spellCheck={false}
            size="large"
            aria-label="Sua senha"
          />
          {errorObject?.key === 'password' && (
            <FormControl.Validation variant="error">{errorObject.message}</FormControl.Validation>
          )}
        </FormControl>
        <FormControl>
          <FormControl.Label visuallyHidden>Criar cadastro</FormControl.Label>
          <Button variant="primary" size="large" type="submit" disabled={isLoading} aria-label="Criar cadastro">
            Criar cadastro
          </Button>
        </FormControl>
      </Box>
    </form>
  );

Additional context (bonus)

I'm in love with Primer. It's been a long time since I've been this happy developing interfaces.

To everyone involved in this project: you are amazing 🤝

@EnixCoda
Copy link

EnixCoda commented Jun 3, 2022

I'm also having this issue. simple reproduction CodeSandbox demo

The issue occurs because the FormControl is using double-render strategy.

/** Slots uses a Double render strategy inspired by [reach-ui/descendants](https://github.com/reach/reach-ui/tree/develop/packages/descendants)
* Slot registers themself with the Slots parent.
* When all the children have mounted = registered themselves in slot,
* we re-render the parent component to render with slots
*/
const Slots: React.FC<{

Only input component was rendered at the first render.

1st render 2nd render
Slots image image
DOM image image

It might be hard to prevent Layout Shift but I hope at least all components can start render since the same render, instead of first render the input then render label or other stuff.

I guess these solutions might work

  1. Wrapping the input component with slot as well, contents would be rendered since the 2nd render.
    {React.isValidElement(InputComponent) &&
    React.cloneElement(InputComponent, {
    id,
    disabled,
    ['aria-describedby']: captionId
    })}
  2. In SlotsContext, manage slots with useState instead of useRef. React would re-render immediately if its state changed within effect. demo This way could potentially prevent layout shift.

Thanks to every contributors of this project, it is amazing!

@siddharthkp
Copy link
Member

siddharthkp commented Jun 3, 2022

Hi!

First of all, sorry @filipedeschamps, we missed your initial message!

Thanks for reporting this. @EnixCoda is right, this is related to double rendered strategy for slots, we are tracking it here: #1690

@iansan5653
Copy link
Contributor

I think this is likely solved by #2216; now both renders should happen before the first paint so there should be no visible flash. I don't see the problem on my end anymore. Please let us know if you do still see it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants