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.

Sign up for GitHub

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

Add blue focus on text inputs #8343

Merged
merged 12 commits into from
Nov 6, 2024
Merged

Add blue focus on text inputs #8343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f165495
Add blue focus on text inputs
FelixMalfait Nov 5, 2024
c66f354
View edit
FelixMalfait Nov 5, 2024
49d1a27
Improve left icon
FelixMalfait Nov 5, 2024
ef9fbe5
Proposal for signup/login form
FelixMalfait Nov 5, 2024
46b6742
Signin form separator
FelixMalfait Nov 5, 2024
7504631
Can we do this without useEffect?
FelixMalfait Nov 5, 2024
3da6919
Looks like it actually works without useEffect?
FelixMalfait Nov 5, 2024
3523c26
Better option signup form
FelixMalfait Nov 5, 2024
fadb4a5
Revert "Better option signup form"
FelixMalfait Nov 6, 2024
1509a8a
Final form after discussion with Thomas
FelixMalfait Nov 6, 2024
476442a
Change dropdown width
FelixMalfait Nov 6, 2024
43b7335
Fix tests
FelixMalfait Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ export const SignInUpForm = () => {
submitSSOEmail,
} = useSignInUp(form);

if (
signInUpStep === SignInUpStep.Init &&
!authProviders.google &&
!authProviders.microsoft &&
!authProviders.sso
) {
continueWithEmail();
}
Comment on lines +76 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: This side effect should be moved to a useEffect hook to avoid potential render loop issues


const toggleSSOMode = () => {
if (signInUpStep === SignInUpStep.SSOEmail) {
continueWithEmail();
Expand Down Expand Up @@ -155,6 +164,9 @@ export const SignInUpForm = () => {
Icon={() => <IconGoogle size={theme.icon.size.lg} />}
title="Continue with Google"
onClick={signInWithGoogle}
variant={
signInUpStep === SignInUpStep.Init ? undefined : 'secondary'
}
fullWidth
/>
<HorizontalSeparator visible={false} />
Expand All @@ -167,6 +179,9 @@ export const SignInUpForm = () => {
Icon={() => <IconMicrosoft size={theme.icon.size.lg} />}
title="Continue with Microsoft"
onClick={signInWithMicrosoft}
variant={
signInUpStep === SignInUpStep.Init ? undefined : 'secondary'
}
fullWidth
/>
<HorizontalSeparator visible={false} />
Expand All @@ -176,6 +191,9 @@ export const SignInUpForm = () => {
<>
<MainButton
Icon={() => <IconKey size={theme.icon.size.lg} />}
variant={
signInUpStep === SignInUpStep.Init ? undefined : 'secondary'
}
title={
signInUpStep === SignInUpStep.SSOEmail
? 'Continue with email'
Expand Down Expand Up @@ -282,9 +300,11 @@ export const SignInUpForm = () => {
</StyledFullWidthMotionDiv>
)}
<MainButton
variant="secondary"
title={buttonTitle}
type="submit"
variant={
signInUpStep === SignInUpStep.Init ? 'secondary' : 'primary'
}
onClick={async () => {
if (signInUpStep === SignInUpStep.Init) {
continueWithEmail();
Expand All @@ -301,7 +321,7 @@ export const SignInUpForm = () => {
setShowErrors(true);
form.handleSubmit(submitCredentials)();
}}
Icon={() => form.formState.isSubmitting && <Loader />}
Icon={() => (form.formState.isSubmitting ? <Loader /> : null)}
disabled={isSubmitButtonDisabled}
fullWidth
/>
Expand Down
2 changes: 1 addition & 1 deletion .../object-filter-dropdown/components/MultipleFiltersDropdownFilterOnFilterChangedEffect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const MultipleFiltersDropdownFilterOnFilterChangedEffect = ({
setDropdownWidth(280);
break;
default:
setDropdownWidth(160);
setDropdownWidth(200);
}
}, [filterDefinitionUsedInDropdownType, setDropdownWidth]);

Expand Down
40 changes: 21 additions & 19 deletions packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
useRef,
useState,
} from 'react';
import { IconComponent, IconEye, IconEyeOff } from 'twenty-ui';
import { IconComponent, IconEye, IconEyeOff, RGBA } from 'twenty-ui';
import { useCombinedRefs } from '~/hooks/useCombinedRefs';
import { turnIntoEmptyStringIfWhitespacesOnly } from '~/utils/string/turnIntoEmptyStringIfWhitespacesOnly';

Expand All @@ -33,6 +33,7 @@ const StyledInputContainer = styled.div`
display: flex;
flex-direction: row;
width: 100%;
position: relative;
`;

const StyledInput = styled.input<
Expand All @@ -42,12 +43,7 @@ const StyledInput = styled.input<
border: 1px solid
${({ theme, error }) =>
error ? theme.border.color.danger : theme.border.color.medium};
border-bottom-left-radius: ${({ theme, LeftIcon }) =>
!LeftIcon && theme.border.radius.sm};
border-right: none;
border-left: ${({ LeftIcon }) => LeftIcon && 'none'};
border-top-left-radius: ${({ theme, LeftIcon }) =>
!LeftIcon && theme.border.radius.sm};
border-radius: ${({ theme }) => theme.border.radius.sm};
box-sizing: border-box;
color: ${({ theme }) => theme.font.color.primary};
display: flex;
Expand All @@ -57,6 +53,8 @@ const StyledInput = styled.input<
height: 32px;
outline: none;
padding: ${({ theme }) => theme.spacing(2)};
padding-left: ${({ theme, LeftIcon }) =>
LeftIcon ? `calc(${theme.spacing(4)} + 16px)` : theme.spacing(2)};
width: 100%;
Comment on lines +56 to 58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The left padding calculation assumes a fixed icon size of 16px. Consider using theme.icon.size.md instead for consistency.


&::placeholder,
Expand All @@ -69,6 +67,13 @@ const StyledInput = styled.input<
&:disabled {
color: ${({ theme }) => theme.font.color.tertiary};
}

&:focus {
${({ theme }) => {
return `box-shadow: 0px 0px 0px 3px ${RGBA(theme.color.blue, 0.1)};
border-color: ${theme.color.blue};`;
}};
}
Comment on lines +71 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The focus state uses a semi-transparent blue shadow (0.1 opacity) which may be too subtle for accessibility. Consider increasing the opacity for better visibility.

`;

const StyledErrorHelper = styled.div`
Expand All @@ -79,30 +84,27 @@ const StyledErrorHelper = styled.div`

const StyledLeftIconContainer = styled.div`
align-items: center;
background-color: ${({ theme }) => theme.background.transparent.lighter};
border: 1px solid ${({ theme }) => theme.border.color.medium};
border-bottom-left-radius: ${({ theme }) => theme.border.radius.sm};
border-right: none;
border-top-left-radius: ${({ theme }) => theme.border.radius.sm};
display: flex;
justify-content: center;
padding-left: ${({ theme }) => theme.spacing(2)};
position: absolute;
top: 0;
bottom: 0;
margin: auto 0;
`;

const StyledTrailingIconContainer = styled.div<
Pick<TextInputV2ComponentProps, 'error'>
>`
align-items: center;
background-color: ${({ theme }) => theme.background.transparent.lighter};
border: 1px solid
${({ theme, error }) =>
error ? theme.border.color.danger : theme.border.color.medium};
border-bottom-right-radius: ${({ theme }) => theme.border.radius.sm};
border-left: none;
border-top-right-radius: ${({ theme }) => theme.border.radius.sm};
display: flex;
justify-content: center;
padding-right: ${({ theme }) => theme.spacing(1)};
position: absolute;
top: 0;
bottom: 0;
right: 0;
margin: auto 0;
`;

const StyledTrailingIcon = styled.div`
Expand Down
2 changes: 1 addition & 1 deletion packages/twenty-front/src/modules/ui/layout/dropdown/components/DropdownMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const StyledDropdownMenu = styled.div<{

flex-direction: column;
z-index: 30;
width: ${({ width = 160 }) =>
width: ${({ width = 200 }) =>
typeof width === 'number' ? `${width}px` : width};
`;

Expand Down
4 changes: 2 additions & 2 deletions packages/twenty-front/src/modules/ui/layout/dropdown/hooks/__tests__/useDropdown.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { act } from 'react-dom/test-utils';
import { expect } from '@storybook/test';
import { renderHook } from '@testing-library/react';
import { act } from 'react-dom/test-utils';
import { RecoilRoot } from 'recoil';

import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown';
Expand Down Expand Up @@ -57,7 +57,7 @@ describe('useDropdown', () => {
wrapper: Wrapper,
});

expect(result.current.dropdownWidth).toBe(160);
expect(result.current.dropdownWidth).toBe(200);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: default width changed from 160 to 200 - ensure this change is intentional and coordinated with the dropdownWidthComponentState default value

await act(async () => {
result.current.setDropdownWidth(220);
Expand Down
2 changes: 1 addition & 1 deletion packages/twenty-front/src/modules/ui/layout/dropdown/states/dropdownWidthComponentState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ export const dropdownWidthComponentState = createComponentState<
number | undefined
>({
key: 'dropdownWidthComponentState',
defaultValue: 160,
defaultValue: 200,
});
8 changes: 4 additions & 4 deletions ...ges/twenty-front/src/modules/views/view-picker/components/ViewPickerContentCreateMode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { IconLayoutKanban, IconTable, IconX } from 'twenty-ui';

import { IconPicker } from '@/ui/input/components/IconPicker';
import { Select } from '@/ui/input/components/Select';
import { TextInputV2 } from '@/ui/input/components/TextInputV2';
import { DropdownMenuHeader } from '@/ui/layout/dropdown/components/DropdownMenuHeader';
import { DropdownMenuInput } from '@/ui/layout/dropdown/components/DropdownMenuInput';
import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer';
import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownMenuSeparator';
import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys';
Expand Down Expand Up @@ -120,11 +120,11 @@ export const ViewPickerContentCreateMode = () => {
disableBlur
onClose={() => setHotkeyScope(ViewsHotkeyScope.ListDropdown)}
/>
<DropdownMenuInput
<TextInputV2
value={viewPickerInputName}
onChange={(event) => {
onChange={(value) => {
setViewPickerIsDirty(true);
setViewPickerInputName(event.target.value);
setViewPickerInputName(value);
}}
autoFocus
/>
Expand Down
8 changes: 4 additions & 4 deletions packages/twenty-front/src/modules/views/view-picker/components/ViewPickerContentEditMode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { Key } from 'ts-key-enum';
import { IconChevronLeft } from 'twenty-ui';

import { IconPicker } from '@/ui/input/components/IconPicker';
import { TextInputV2 } from '@/ui/input/components/TextInputV2';
import { DropdownMenuHeader } from '@/ui/layout/dropdown/components/DropdownMenuHeader';
import { DropdownMenuInput } from '@/ui/layout/dropdown/components/DropdownMenuInput';
import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer';
import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownMenuSeparator';
import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys';
Expand Down Expand Up @@ -79,11 +79,11 @@ export const ViewPickerContentEditMode = () => {
disableBlur
onClose={() => setHotkeyScope(ViewsHotkeyScope.ListDropdown)}
/>
<DropdownMenuInput
<TextInputV2
value={viewPickerInputName}
onChange={(event) => {
onChange={(value) => {
setViewPickerIsDirty(true);
setViewPickerInputName(event.target.value);
setViewPickerInputName(value);
}}
autoFocus
/>
Comment on lines +82 to 89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: TextInputV2 may need a placeholder prop to match previous DropdownMenuInput behavior

Expand Down
4 changes: 2 additions & 2 deletions packages/twenty-front/src/pages/auth/SignInUp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { SignInUpForm } from '@/auth/sign-in-up/components/SignInUpForm';
import { SignInUpMode, useSignInUp } from '@/auth/sign-in-up/hooks/useSignInUp';
import { useSignInUpForm } from '@/auth/sign-in-up/hooks/useSignInUpForm';
import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState';
import { AnimatedEaseIn } from 'twenty-ui';
import { isDefined } from '~/utils/isDefined';
import { SignInUpStep } from '@/auth/states/signInUpStepState';
import { IconLockCustom } from '@ui/display/icon/components/IconLock';
import { AnimatedEaseIn } from 'twenty-ui';
import { isDefined } from '~/utils/isDefined';
import { SSOWorkspaceSelection } from './SSOWorkspaceSelection';

export const SignInUp = () => {
Expand Down
Loading