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

Jump to bottom

Fix logic about props 'emails' #132

Merged
merged 3 commits into from
Jul 17, 2023
Merged

Fix logic about props 'emails' #132

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5c9e35
Fix logic about props 'emails'
Jul 16, 2023
100f397
Fix check validation about props 'emails'
Jul 16, 2023
2c3466f
Fix check validation about props 'emails'
Jul 16, 2023
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
28 changes: 16 additions & 12 deletions react-multi-email/ReactMultiEmail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { isEmail as isEmailFn } from './isEmail';
import { isEmail, isEmail as isEmailFn } from './isEmail';

export interface IReactMultiEmailProps {
id?: string;
Expand Down Expand Up @@ -31,15 +31,27 @@ export interface IReactMultiEmailProps {
autoComplete?: string;
}

const initialEmailAddress = (emails?: string[]) => {
if (typeof emails === 'undefined') return [];

const validEmails = emails.filter(email => isEmail(email));
return validEmails;
};

export function ReactMultiEmail(props: IReactMultiEmailProps) {
const {
id,
style,
getLabel,
emails: propsEmails,
className = '',
noClass,
placeholder,
autoFocus,
delimiter = '[ ,;]',
initialInputValue = '',
inputClassName,
autoComplete,
getLabel,
enable,
onDisabled,
validateEmail,
Expand All @@ -50,22 +62,18 @@ export function ReactMultiEmail(props: IReactMultiEmailProps) {
onKeyDown,
onKeyUp,
spinner,
delimiter = '[ ,;]',
initialInputValue = '',
inputClassName,
autoComplete,
} = props;
const emailInputRef = React.useRef<HTMLInputElement>(null);

const [focused, setFocused] = React.useState(false);
const [emails, setEmails] = React.useState<string[]>([]);
const [emails, setEmails] = React.useState<string[]>(() => initialEmailAddress(propsEmails));
Copy link
Member

Choose a reason for hiding this comment

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

이런식도 가능 했네요. state를 많이 안쓰다보니. 오늘 하나 배워 갑니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

추가로 덧붙이자면..
얼마전까지 제가 많이 실수하던 코드인데

// 1번 - BAD
const [selected, setSelected] = useState(getIsSelected(id));

// 2번 - GOOD
const [selected, setSelected] = useState(() => getIsSelected(id));

의도 자체는 랜더링시에 한번만 getIsSelected를 실행시키기 위한 코드지만 1번과 같이 짤경우 랜더링시마다 getIsSelected 를 실행하게 되어 getIsSelected로직이 복잡할 경우 성능에 영향을 줄 수 있다고 합니다.

const [inputValue, setInputValue] = React.useState(initialInputValue);
const [spinning, setSpinning] = React.useState(false);

const findEmailAddress = React.useCallback(
async (value: string, isEnter?: boolean) => {
const validEmails: string[] = [];
let inputValue: string = '';
let inputValue = '';
const re = new RegExp(delimiter, 'g');
const isEmail = validateEmail || isEmailFn;

Expand Down Expand Up @@ -235,10 +243,6 @@ export function ReactMultiEmail(props: IReactMultiEmailProps) {
onFocus?.();
}, [onFocus]);

React.useEffect(() => {
setEmails(props.emails ?? []);
}, [props.emails]);

return (
<div
className={`${className} ${noClass ? '' : 'react-multi-email'} ${focused ? 'focused' : ''} ${
Expand Down