From 17422b7690f52274dc62820e08681638e91514d9 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 17 Apr 2024 10:45:02 +0200 Subject: [PATCH] fix: I should be able to use "enter" key to create profile (#4978) ## Context Fixes #4808 TL;DR Introducing pure stateless modal component ("UI modal") for our auth modal not to have default hotkeyScope overriding our create-profile hotkeyScope + we dont want the shortcut to be available for all the modal content, only for the input that should not be using a hotkeyscope, so we are using onKeyDown for the specific issue on create profile. Explanation create-profile hotkey scope is set by PageChangeEffect; CreateProfile component adds enter key shortcut; but this scope is overwritten by the default scope by the Modal component that expects a hotkeyScope to reset to (and defaults to the default hotkeyScope if none indicated). In the auth flow we were using that Modal component to give a modal look to the flow but it is not a modal per say, it's a set of pages contained within a modal look. By creating this UI component we are escaping that hotkeyScope overriding that does not make sense in our context. ## How was it tested Locally Storybook --- .../src/modules/auth/components/Modal.tsx | 8 +- .../sign-in-up/components/SignInUpForm.tsx | 4 +- .../auth/sign-in-up/hooks/useSignInUp.tsx | 27 --- .../ui/layout/modal/components/Modal.tsx | 182 +++--------------- .../layout/modal/components/ModalLayout.tsx | 168 ++++++++++++++++ .../__stories__/ModalLayout.stories.tsx | 36 ++++ .../src/pages/auth/CreateProfile.tsx | 21 +- 7 files changed, 244 insertions(+), 202 deletions(-) create mode 100644 packages/twenty-front/src/modules/ui/layout/modal/components/ModalLayout.tsx create mode 100644 packages/twenty-front/src/modules/ui/layout/modal/components/__stories__/ModalLayout.stories.tsx diff --git a/packages/twenty-front/src/modules/auth/components/Modal.tsx b/packages/twenty-front/src/modules/auth/components/Modal.tsx index 3a9429270..f40de47cf 100644 --- a/packages/twenty-front/src/modules/auth/components/Modal.tsx +++ b/packages/twenty-front/src/modules/auth/components/Modal.tsx @@ -1,9 +1,9 @@ import React from 'react'; import styled from '@emotion/styled'; -import { Modal as UIModal } from '@/ui/layout/modal/components/Modal'; +import { ModalLayout } from '@/ui/layout/modal/components/ModalLayout'; -const StyledContent = styled(UIModal.Content)` +const StyledContent = styled(ModalLayout.Content)` align-items: center; width: calc(400px - ${({ theme }) => theme.spacing(10 * 2)}); `; @@ -11,7 +11,7 @@ const StyledContent = styled(UIModal.Content)` type AuthModalProps = { children: React.ReactNode }; export const AuthModal = ({ children }: AuthModalProps) => ( - + {children} - + ); diff --git a/packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx b/packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx index 6aa443e24..a183ff4be 100644 --- a/packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx +++ b/packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx @@ -164,9 +164,9 @@ export const SignInUpForm = () => { } }} error={showErrors ? error?.message : undefined} - onKeyDown={handleKeyDown} fullWidth disableHotkeys + onKeyDown={handleKeyDown} /> )} @@ -198,10 +198,10 @@ export const SignInUpForm = () => { placeholder="Password" onBlur={onBlur} onChange={onChange} - onKeyDown={handleKeyDown} error={showErrors ? error?.message : undefined} fullWidth disableHotkeys + onKeyDown={handleKeyDown} /> )} diff --git a/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.tsx b/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.tsx index c14994363..4fa77f3d5 100644 --- a/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.tsx +++ b/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.tsx @@ -5,9 +5,7 @@ import { useParams } from 'react-router-dom'; import { useNavigateAfterSignInUp } from '@/auth/sign-in-up/hooks/useNavigateAfterSignInUp.ts'; import { Form } from '@/auth/sign-in-up/hooks/useSignInUpForm.ts'; import { AppPath } from '@/types/AppPath'; -import { PageHotkeyScope } from '@/types/PageHotkeyScope'; import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; -import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; import { useAuth } from '../../hooks/useAuth'; @@ -118,31 +116,6 @@ export const useSignInUp = (form: UseFormReturn
) => { ], ); - useScopedHotkeys( - 'enter', - () => { - if (signInUpStep === SignInUpStep.Init) { - continueWithEmail(); - } - - if (signInUpStep === SignInUpStep.Email) { - continueWithCredentials(); - } - - if (signInUpStep === SignInUpStep.Password) { - form.handleSubmit(submitCredentials)(); - } - }, - PageHotkeyScope.SignInUp, - [ - continueWithEmail, - signInUpStep, - continueWithCredentials, - form, - submitCredentials, - ], - ); - return { isInviteMode, signInUpStep, diff --git a/packages/twenty-front/src/modules/ui/layout/modal/components/Modal.tsx b/packages/twenty-front/src/modules/ui/layout/modal/components/Modal.tsx index 9d40af2ce..445086b52 100644 --- a/packages/twenty-front/src/modules/ui/layout/modal/components/Modal.tsx +++ b/packages/twenty-front/src/modules/ui/layout/modal/components/Modal.tsx @@ -1,8 +1,10 @@ import React, { useEffect, useRef } from 'react'; -import styled from '@emotion/styled'; -import { motion } from 'framer-motion'; import { Key } from 'ts-key-enum'; +import { + ModalLayout, + ModalLayoutProps, +} from '@/ui/layout/modal/components/ModalLayout'; import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope'; import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { @@ -12,135 +14,11 @@ import { import { ModalHotkeyScope } from './types/ModalHotkeyScope'; -const StyledModalDiv = styled(motion.div)<{ - size?: ModalSize; - padding?: ModalPadding; -}>` - display: flex; - flex-direction: column; - background: ${({ theme }) => theme.background.primary}; - color: ${({ theme }) => theme.font.color.primary}; - border-radius: ${({ theme }) => theme.border.radius.md}; - overflow: hidden; - max-height: 90vh; - z-index: 10000; // should be higher than Backdrop's z-index - - width: ${({ size, theme }) => { - switch (size) { - case 'small': - return theme.modal.size.sm; - case 'medium': - return theme.modal.size.md; - case 'large': - return theme.modal.size.lg; - default: - return 'auto'; - } - }}; - - padding: ${({ padding, theme }) => { - switch (padding) { - case 'none': - return theme.spacing(0); - case 'small': - return theme.spacing(2); - case 'medium': - return theme.spacing(4); - case 'large': - return theme.spacing(6); - default: - return 'auto'; - } - }}; -`; - -const StyledHeader = styled.div` - align-items: center; - display: flex; - flex-direction: row; - height: 60px; - overflow: hidden; - padding: ${({ theme }) => theme.spacing(5)}; -`; - -const StyledContent = styled.div` - display: flex; - flex: 1; - flex: 1 1 0%; - flex-direction: column; - overflow-y: auto; - padding: ${({ theme }) => theme.spacing(10)}; -`; - -const StyledFooter = styled.div` - align-items: center; - display: flex; - flex-direction: row; - height: 60px; - overflow: hidden; - padding: ${({ theme }) => theme.spacing(5)}; -`; - -const StyledBackDrop = styled(motion.div)` - align-items: center; - background: ${({ theme }) => theme.background.overlay}; - display: flex; - height: 100%; - justify-content: center; - left: 0; - position: fixed; - top: 0; - width: 100%; - z-index: 9999; -`; - -/** - * Modal components - */ -type ModalHeaderProps = React.PropsWithChildren & { - className?: string; -}; - -const ModalHeader = ({ children, className }: ModalHeaderProps) => ( - {children} -); - -type ModalContentProps = React.PropsWithChildren & { - className?: string; -}; - -const ModalContent = ({ children, className }: ModalContentProps) => ( - {children} -); - -type ModalFooterProps = React.PropsWithChildren & { - className?: string; -}; - -const ModalFooter = ({ children, className }: ModalFooterProps) => ( - {children} -); - -/** - * Modal - */ -export type ModalSize = 'small' | 'medium' | 'large'; -export type ModalPadding = 'none' | 'small' | 'medium' | 'large'; - -type ModalProps = React.PropsWithChildren & { +type ModalProps = ModalLayoutProps & { isOpen?: boolean; - onClose?: () => void; hotkeyScope?: ModalHotkeyScope; + onClose?: () => void; onEnter?: () => void; - size?: ModalSize; - padding?: ModalPadding; - className?: string; -}; - -const modalVariants = { - hidden: { opacity: 0 }, - visible: { opacity: 1 }, - exit: { opacity: 0 }, }; export const Modal = ({ @@ -153,14 +31,6 @@ export const Modal = ({ padding = 'medium', className, }: ModalProps) => { - const modalRef = useRef(null); - - useListenClickOutside({ - refs: [modalRef], - callback: () => onClose?.(), - mode: ClickOutsideMode.comparePixels, - }); - const { goBackToPreviousHotkeyScope, setHotkeyScopeAndMemorizePreviousScope, @@ -196,30 +66,28 @@ export const Modal = ({ setHotkeyScopeAndMemorizePreviousScope, ]); + const modalRef = useRef(null); + + useListenClickOutside({ + refs: [modalRef], + callback: () => onClose?.(), + mode: ClickOutsideMode.comparePixels, + }); + return isOpen ? ( - - - {children} - - + + {children} + ) : ( <> ); }; -Modal.Header = ModalHeader; -Modal.Content = ModalContent; -Modal.Footer = ModalFooter; +Modal.Header = ModalLayout.Header; +Modal.Content = ModalLayout.Content; +Modal.Footer = ModalLayout.Footer; diff --git a/packages/twenty-front/src/modules/ui/layout/modal/components/ModalLayout.tsx b/packages/twenty-front/src/modules/ui/layout/modal/components/ModalLayout.tsx new file mode 100644 index 000000000..c6437c247 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/layout/modal/components/ModalLayout.tsx @@ -0,0 +1,168 @@ +import React from 'react'; +import styled from '@emotion/styled'; +import { motion } from 'framer-motion'; + +const StyledModalDiv = styled(motion.div)<{ + size?: ModalSize; + padding?: ModalPadding; +}>` + display: flex; + flex-direction: column; + background: ${({ theme }) => theme.background.primary}; + color: ${({ theme }) => theme.font.color.primary}; + border-radius: ${({ theme }) => theme.border.radius.md}; + overflow: hidden; + max-height: 90vh; + z-index: 10000; // should be higher than Backdrop's z-index + + width: ${({ size, theme }) => { + switch (size) { + case 'small': + return theme.modal.size.sm; + case 'medium': + return theme.modal.size.md; + case 'large': + return theme.modal.size.lg; + default: + return 'auto'; + } + }}; + + padding: ${({ padding, theme }) => { + switch (padding) { + case 'none': + return theme.spacing(0); + case 'small': + return theme.spacing(2); + case 'medium': + return theme.spacing(4); + case 'large': + return theme.spacing(6); + default: + return 'auto'; + } + }}; +`; + +const StyledHeader = styled.div` + align-items: center; + display: flex; + flex-direction: row; + height: 60px; + overflow: hidden; + padding: ${({ theme }) => theme.spacing(5)}; +`; + +const StyledContent = styled.div` + display: flex; + flex: 1; + flex: 1 1 0%; + flex-direction: column; + overflow-y: auto; + padding: ${({ theme }) => theme.spacing(10)}; +`; + +const StyledFooter = styled.div` + align-items: center; + display: flex; + flex-direction: row; + height: 60px; + overflow: hidden; + padding: ${({ theme }) => theme.spacing(5)}; +`; + +const StyledBackDrop = styled(motion.div)` + align-items: center; + background: ${({ theme }) => theme.background.overlay}; + display: flex; + height: 100%; + justify-content: center; + left: 0; + position: fixed; + top: 0; + width: 100%; + z-index: 9999; +`; + +/** + * Modal components + */ +type ModalLayoutHeaderProps = React.PropsWithChildren & { + className?: string; +}; + +const ModalLayoutHeader = ({ children, className }: ModalLayoutHeaderProps) => ( + {children} +); + +type ModalLayoutContentProps = React.PropsWithChildren & { + className?: string; +}; + +const ModalLayoutContent = ({ + children, + className, +}: ModalLayoutContentProps) => ( + {children} +); + +type ModalLayoutFooterProps = React.PropsWithChildren & { + className?: string; +}; + +const ModalLayoutFooter = ({ children, className }: ModalLayoutFooterProps) => ( + {children} +); + +/** + * Modal + */ +export type ModalSize = 'small' | 'medium' | 'large'; +export type ModalPadding = 'none' | 'small' | 'medium' | 'large'; + +export type ModalLayoutProps = React.PropsWithChildren & { + size?: ModalSize; + padding?: ModalPadding; + className?: string; + modalRef?: React.RefObject; +}; + +const modalVariants = { + hidden: { opacity: 0 }, + visible: { opacity: 1 }, + exit: { opacity: 0 }, +}; + +// This component should be used over Modal when seeking a modal feel without modal state (hotkeyScope etc) +export const ModalLayout = ({ + children, + size = 'medium', + padding = 'medium', + modalRef, + className, +}: ModalLayoutProps) => { + return ( + + + {children} + + + ); +}; + +ModalLayout.Header = ModalLayoutHeader; +ModalLayout.Content = ModalLayoutContent; +ModalLayout.Footer = ModalLayoutFooter; diff --git a/packages/twenty-front/src/modules/ui/layout/modal/components/__stories__/ModalLayout.stories.tsx b/packages/twenty-front/src/modules/ui/layout/modal/components/__stories__/ModalLayout.stories.tsx new file mode 100644 index 000000000..538c7b299 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/layout/modal/components/__stories__/ModalLayout.stories.tsx @@ -0,0 +1,36 @@ +import { Meta, StoryObj } from '@storybook/react'; +import { ComponentDecorator } from 'twenty-ui'; + +import { ModalLayout } from '@/ui/layout/modal/components/ModalLayout'; + +const meta: Meta = { + title: 'UI/Layout/Modal/ModalLayout', + component: ModalLayout, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + args: { + size: 'medium', + padding: 'medium', + children: ( + <> + Stay in touch + + This is a dummy newletter form so don't bother trying to test it. Not + that I expect you to, anyways. :) + + + By using Twenty, you're opting for the finest CRM experience you'll + ever encounter. + + + ), + }, + decorators: [ComponentDecorator], + argTypes: { + children: { control: false }, + }, +}; diff --git a/packages/twenty-front/src/pages/auth/CreateProfile.tsx b/packages/twenty-front/src/pages/auth/CreateProfile.tsx index edbaa99bc..a5e76d433 100644 --- a/packages/twenty-front/src/pages/auth/CreateProfile.tsx +++ b/packages/twenty-front/src/pages/auth/CreateProfile.tsx @@ -3,7 +3,6 @@ import { Controller, SubmitHandler, useForm } from 'react-hook-form'; import styled from '@emotion/styled'; import { zodResolver } from '@hookform/resolvers/zod'; import { useRecoilState } from 'recoil'; -import { Key } from 'ts-key-enum'; import { z } from 'zod'; import { SubTitle } from '@/auth/components/SubTitle'; @@ -14,12 +13,10 @@ import { OnboardingStatus } from '@/auth/utils/getOnboardingStatus'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord'; import { ProfilePictureUploader } from '@/settings/profile/components/ProfilePictureUploader'; -import { PageHotkeyScope } from '@/types/PageHotkeyScope'; import { H2Title } from '@/ui/display/typography/components/H2Title'; import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { MainButton } from '@/ui/input/button/components/MainButton'; import { TextInput } from '@/ui/input/components/TextInput'; -import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { WorkspaceMember } from '@/workspace-member/types/WorkspaceMember'; const StyledContentContainer = styled.div` @@ -126,19 +123,17 @@ export const CreateProfile = () => { ], ); - useScopedHotkeys( - Key.Enter, - () => { - onSubmit(getValues()); - }, - PageHotkeyScope.CreateProfile, - [onSubmit], - ); - if (onboardingStatus !== OnboardingStatus.OngoingProfileCreation) { return null; } + const onNameInputKeyDown = (event: React.KeyboardEvent) => { + if (event.key === 'Enter') { + event.preventDefault(); + onSubmit(getValues()); + } + }; + return ( <> Create profile @@ -171,6 +166,7 @@ export const CreateProfile = () => { placeholder="Tim" error={error?.message} fullWidth + onKeyDown={onNameInputKeyDown} disableHotkeys /> )} @@ -190,6 +186,7 @@ export const CreateProfile = () => { placeholder="Cook" error={error?.message} fullWidth + onKeyDown={onNameInputKeyDown} disableHotkeys /> )}