Skip to content

Commit

Permalink
Use <label> HTML element when possible (#7609)
Browse files Browse the repository at this point in the history
This PR:

- Uses `<label>` HTML elements when possible to represent labels
- Uses the new `useId()` React hook to get an identifier to link the
label with its input; it's more suitable than generating a UUID at every
render

Fixes #7281

---------

Co-authored-by: Charles Bochet <[email protected]>
  • Loading branch information
Devessier and charlesBochet authored Oct 13, 2024
1 parent 284b267 commit 05e8f8a
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import styled from '@emotion/styled';
import React, { useId } from 'react';

interface TextInputProps {
label?: string;
Expand All @@ -18,7 +18,7 @@ const StyledContainer = styled.div<{ fullWidth?: boolean }>`
margin-bottom: ${({ theme }) => theme.spacing(4)};
`;

const StyledLabel = styled.span`
const StyledLabel = styled.label`
color: ${({ theme }) => theme.font.color.light};
font-size: ${({ theme }) => theme.font.size.xs};
font-weight: ${({ theme }) => theme.font.weight.semiBold};
Expand Down Expand Up @@ -65,12 +65,15 @@ const TextInput: React.FC<TextInputProps> = ({
placeholder,
icon,
}) => {
const inputId = useId();

return (
<StyledContainer fullWidth={fullWidth}>
{label && <StyledLabel>{label}</StyledLabel>}
{label && <StyledLabel htmlFor={inputId}>{label}</StyledLabel>}
<StyledInputContainer>
{icon && <StyledIcon>{icon}</StyledIcon>}
<StyledInput
id={inputId}
type="text"
value={value}
onChange={(e) => onChange(e.target.value)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import styled from '@emotion/styled';
import * as React from 'react';
import { IconCheck, IconMinus } from 'twenty-ui';
import { v4 } from 'uuid';

export enum CheckboxVariant {
Primary = 'primary',
Expand Down Expand Up @@ -165,7 +164,7 @@ export const Checkbox = ({
setIsInternalChecked(event.target.checked ?? false);
};

const checkboxId = 'checkbox' + v4();
const checkboxId = React.useId();

return (
<StyledInputContainer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { motion } from 'framer-motion';
import * as React from 'react';
import { RGBA } from 'twenty-ui';

import { v4 } from 'uuid';
import { RadioGroup } from './RadioGroup';

export enum RadioSize {
Expand Down Expand Up @@ -134,7 +133,7 @@ export const Radio = ({
onCheckedChange?.(event.target.checked);
};

const optionId = v4();
const optionId = React.useId();

return (
<StyledContainer className={className} labelPosition={labelPosition}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ForwardedRef,
InputHTMLAttributes,
forwardRef,
useId,
useRef,
useState,
} from 'react';
Expand All @@ -21,7 +22,7 @@ const StyledContainer = styled.div<
width: ${({ fullWidth }) => (fullWidth ? `100%` : 'auto')};
`;

const StyledLabel = styled.span`
const StyledLabel = styled.label`
color: ${({ theme }) => theme.font.color.light};
font-size: ${({ theme }) => theme.font.size.xs};
font-weight: ${({ theme }) => theme.font.weight.semiBold};
Expand Down Expand Up @@ -169,9 +170,15 @@ const TextInputV2Component = (
setPasswordVisible(!passwordVisible);
};

const inputId = useId();

return (
<StyledContainer className={className} fullWidth={fullWidth ?? false}>
{label && <StyledLabel>{label + (required ? '*' : '')}</StyledLabel>}
{label && (
<StyledLabel htmlFor={inputId}>
{label + (required ? '*' : '')}
</StyledLabel>
)}
<StyledInputContainer>
{!!LeftIcon && (
<StyledLeftIconContainer>
Expand All @@ -181,6 +188,7 @@ const TextInputV2Component = (
</StyledLeftIconContainer>
)}
<StyledInput
id={inputId}
data-testid={dataTestId}
autoComplete={autoComplete || 'off'}
ref={combinedRef}
Expand Down
13 changes: 8 additions & 5 deletions packages/twenty-front/src/modules/ui/input/components/Toggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,14 @@ export const Toggle = ({
};

useEffect(() => {
if (value !== isOn) {
setIsOn(value ?? false);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [value]);
setIsOn((isOn) => {
if (value !== isOn) {
return value ?? false;
}

return isOn;
});
}, [value, setIsOn]);

return (
<StyledContainer
Expand Down

0 comments on commit 05e8f8a

Please sign in to comment.