From 0b0225dc3d8d187b0c04ced9df49eba1fb86da3c Mon Sep 17 00:00:00 2001 From: Devessier Date: Fri, 11 Oct 2024 18:16:19 +0200 Subject: [PATCH 1/3] fix: use a true label element for the input label --- .../modules/ui/input/components/TextInput.tsx | 9 ++++++--- .../src/modules/ui/input/components/TextInputV2.tsx | 12 ++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/twenty-chrome-extension/src/options/modules/ui/input/components/TextInput.tsx b/packages/twenty-chrome-extension/src/options/modules/ui/input/components/TextInput.tsx index 1b1af1e59ac0..760b7625ee2a 100644 --- a/packages/twenty-chrome-extension/src/options/modules/ui/input/components/TextInput.tsx +++ b/packages/twenty-chrome-extension/src/options/modules/ui/input/components/TextInput.tsx @@ -1,5 +1,5 @@ -import React from 'react'; import styled from '@emotion/styled'; +import React, { useId } from 'react'; interface TextInputProps { label?: string; @@ -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}; @@ -65,12 +65,15 @@ const TextInput: React.FC = ({ placeholder, icon, }) => { + const inputId = useId(); + return ( - {label && {label}} + {label && {label}} {icon && {icon}} onChange(e.target.value)} diff --git a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx index cf4b06f6aba9..1f64b2adc55b 100644 --- a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx @@ -6,6 +6,7 @@ import { ForwardedRef, InputHTMLAttributes, forwardRef, + useId, useRef, useState, } from 'react'; @@ -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}; @@ -169,9 +170,15 @@ const TextInputV2Component = ( setPasswordVisible(!passwordVisible); }; + const inputId = useId(); + return ( - {label && {label + (required ? '*' : '')}} + {label && ( + + {label + (required ? '*' : '')} + + )} {!!LeftIcon && ( @@ -181,6 +188,7 @@ const TextInputV2Component = ( )} Date: Fri, 11 Oct 2024 18:17:41 +0200 Subject: [PATCH 2/3] fix: use useId hook instead of a uuid v4 for the id of an input element UUID are cryptographically secure unique identifiers and take time to get generated. It's better to use the useId hook which is made for that purpose, always returns the same value for the same hook instance, and works well with server-side rendering. --- .../twenty-front/src/modules/ui/input/components/Checkbox.tsx | 3 +-- .../twenty-front/src/modules/ui/input/components/Radio.tsx | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/twenty-front/src/modules/ui/input/components/Checkbox.tsx b/packages/twenty-front/src/modules/ui/input/components/Checkbox.tsx index 103036b7cfd7..fd3327c63f23 100644 --- a/packages/twenty-front/src/modules/ui/input/components/Checkbox.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/Checkbox.tsx @@ -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', @@ -165,7 +164,7 @@ export const Checkbox = ({ setIsInternalChecked(event.target.checked ?? false); }; - const checkboxId = 'checkbox' + v4(); + const checkboxId = React.useId(); return ( From 7e42c73dd2675019d69b75453bedf421ce60d48b Mon Sep 17 00:00:00 2001 From: Devessier Date: Fri, 11 Oct 2024 18:18:26 +0200 Subject: [PATCH 3/3] refactor: provide a function setter to the setIsOn function to stop disabling an eslint rule --- .../src/modules/ui/input/components/Toggle.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/twenty-front/src/modules/ui/input/components/Toggle.tsx b/packages/twenty-front/src/modules/ui/input/components/Toggle.tsx index eeceaee8934e..39bafac99582 100644 --- a/packages/twenty-front/src/modules/ui/input/components/Toggle.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/Toggle.tsx @@ -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 (