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.

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

#773 채팅 140자 넘으면 입력 막기 & 글자 수 circular progress bar로 표시 #777

Merged
1 change: 1 addition & 0 deletions packages/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"prop-types": "^15.8.1",
"qs": "^6.11.0",
"react": "^18.2.0",
"react-circular-progressbar": "^2.1.0",
"react-cookie": "^4.1.1",
"react-dom": "^18.2.0",
"react-ga4": "^1.4.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ import theme from "@/tools/theme";

type BodyTextProps = {
sendMessage: ReturnType<typeof useSendMessage>;
onTextChange: (msgLength: number) => void; // 글자 수를 부모에게 전달하여 circular progressbar에 사용
maxChatMsgLength: number; // 채팅 입력 최대 길이입니다.
};

const BodyText = ({ sendMessage }: BodyTextProps) => {
const BodyText = ({
sendMessage,
onTextChange,
maxChatMsgLength,
}: BodyTextProps) => {
const wrapRef = useRef<HTMLDivElement>(null);
const textareaRef = useRef<HTMLTextAreaElement>();
const [height, setHeight] = useState<CSS["height"]>("32px");
Expand Down Expand Up @@ -45,8 +51,13 @@ const BodyText = ({ sendMessage }: BodyTextProps) => {
const [isMessageValidState, setIsMessageValidState] =
useState<boolean>(false);
const getIsMessageValid = useCallback(
(message: string): boolean =>
regExpTest.chatMsg(message) && !isSendingMessage,
(message: string): boolean => {
return (
regExpTest.chatMsg(message) &&
regExpTest.chatMsgLength(message) &&
!isSendingMessage
);
},
[isSendingMessage]
);
useEffect(
Expand Down Expand Up @@ -82,6 +93,15 @@ const BodyText = ({ sendMessage }: BodyTextProps) => {
if (isSendingMessage) refreshTextArea();
setIsMessageValidState(getIsMessageValid(textareaRef.current.value));

if (textareaRef.current.value.length > maxChatMsgLength) {
Copy link
Contributor

Choose a reason for hiding this comment

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

regExpTest.chatMsgLength와 같은 역할 아닌가용? 중복된 로직같아서 여쭤봅니다ㅎㅎ

Choose a reason for hiding this comment

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

@GrasshopperBears 헉 오랜만에 리뷰하신거 보니 반갑구만요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

너무 늦게 확인했네요ㅠㅠ 리뷰 감사합니다!
정규식을 다시 활용하면 BodyText에서 max 길이를 props로 받아올 필요가 없겠네요! 반영하여 수정하겠습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

검사 로직은 기존의 것을 사용하는 것으로 수정하였고, substring 부분의 경우 regExpTest.js에 새로운 함수를 도입하여 match 를 사용하는 것을 시도하였으나 버그가 있어서 우선 검사 로직만 수정한 커밋 올렸습니다
BodyText의 props에서 maxChatLength를 아예 제외한다고 해도, CircularProgressBar때문에 InputText의 index.tsx에 max 길이 변수가 존재해야 하기 때문에, 전달되는 props가 하나 줄어드는 차이 뿐임을 생각하면, 이 정도로 두어도 괜찮을 것 같기도 한데 다들 어떻게 생각하시는지 궁금합니다!

Copy link
Contributor

@GrasshopperBears GrasshopperBears Apr 24, 2024

Choose a reason for hiding this comment

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

제가 본 부분에 한해서 동작상에 문제는 없을 것 같아 일정이 있다면 이하 커멘트는 무시하셔도 괜찮습니다ㅎㅎ

추가적인 커멘트로는

  • (원래 커멘트의 의도) if절 앞 getIsMessageValid에서도 길이 검사를 하는데 여기서 또 한다. 어떻게 메서드를 분리하거나 하나를 생략할 수는 없을지?
  • @jinhyeonkwon 님의 커멘트에서 (개인적으로는) props 전달은 최소화되면 좋다고 생각합니다. 하나 정도는 괜찮지가 쌓여서 props 20개씩 될 수도 있는 거고 그러면 관리가 복잡해지는... 또 역할이라는 측면에서도 생각해볼 수 있을 것 같아요. BodyText의 역할은 무엇인지? 만약 BodyText의 역할이 순수한 텍스트의 표시라면 maxChatLength를 전달해 최대 길이 제한을 여기서 하는 게 맞을까? 길이 제한은 래핑하는 컴포넌트로 할 수도 있지 않을까? (이렇게 한다면 depth가 깊어지는 문제가 생기겠군요; 생각해보니 별로인 것 같은 아이디어인데 암튼) 답은 없을테니 팀원들이랑 잘 고민해보세요ㅎㅎ

textareaRef.current.value = textareaRef.current.value.substring(
0,
maxChatMsgLength
);
}

onTextChange(textareaRef.current.value.length);

if (isEnterPressed.current && !isShiftPressed.current) {
onSend();
return;
Expand Down Expand Up @@ -110,6 +130,7 @@ const BodyText = ({ sendMessage }: BodyTextProps) => {
if (textareaRef.current) wrapRef.current.removeChild(textareaRef.current);
const textarea = document.createElement("textarea");
textarea.oninput = onChange;
// textarea.maxLength = maxChatMsgLength;
textarea.addEventListener("keydown", onKeyDown);
textarea.addEventListener("keyup", onKeyUp);
textarea.placeholder = "채팅을 입력해주세요";
Expand Down
65 changes: 64 additions & 1 deletion packages/web/src/components/Chat/MessageForm/InputText/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { useState } from "react";
import { CircularProgressbar, buildStyles } from "react-circular-progressbar";
import "react-circular-progressbar/dist/styles.css";
Copy link

Choose a reason for hiding this comment

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

흠... 저의 개인적인 생각인데.. 원형 바 만드는데 라이브러리까지 쓸 필요가 있나 싶네요..ㅋㅋㅋ 하하

Copy link
Contributor Author

Choose a reason for hiding this comment

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

가장 쉬운 방법을 사용하긴 했는데, 다른 선택지도 있으면 환영합니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

기존에 있는 라이브러리 쓰는 것도 방법일 것 같아요ㅎㅎ


import useSendMessage from "@/hooks/chat/useSendMessage";

import BodyImage from "./BodyImage";
Expand All @@ -17,8 +21,67 @@ const InputText = ({
sendMessage,
}: InputTextProps) => {
const inputMode = uploadedImage ? "image" : "text";
const [chatMsgLength, setChatMsgLength] = useState<number>(0);
const maxChatMsgLength = 140; // 채팅 입력 최대 길이입니다.
const children = {
text: <BodyText sendMessage={sendMessage} />,
text: (
<div
style={{
display: "flex",
flex: 1,
flexDirection: "row",
alignItems: "center",
}}
Copy link

Choose a reason for hiding this comment

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

저희는 emotion을 사용합니다 !!
css Prop을 사용해주세요!!

style prop에 object로 스타일을 넘기면 react에서 다른 prop 이 정의된 걸로 이해해서
매번 리렌더링 됩니다.. 성능 이슈!!

emotion을 사용하면 css-in-js 여서 오브젝트로 넘겨도 해싱된 className으로 변환해 주어서 괜찮습니다 !!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 그렇군요.. 관련하여 css 부분 수정하도록 하겠습니다..!

>
<div
style={{
width: "28px",
height: "28px",
marginLeft: "0px",
position: "relative",
}}
Copy link

Choose a reason for hiding this comment

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

마찬가지!

>
<CircularProgressbar
value={chatMsgLength}
maxValue={maxChatMsgLength}
strokeWidth={10}
styles={buildStyles({
// Rotation of path and trail, in number of turns (0-1)
rotation: 0.25,

// Whether to use rounded or flat corners on the ends - can use 'butt' or 'round'
strokeLinecap: "butt",

// Text size
textSize: "28px",

// How long animation takes to go from one chatMsgLength to another, in seconds
pathTransitionDuration: 0.2,

// Can specify path transition in more detail, or remove it entirely
// pathTransition: 'none',

// Colors
pathColor: `${
chatMsgLength < maxChatMsgLength
? `rgba(110, 54, 120)`
: `rgba(180, 0, 0)`
}`,
textColor: "#000000",
trailColor: "#d6d6d6",
backgroundColor: "#3e98c7",
})}
/>
</div>
<div style={{ flex: 1 }}>
Copy link

Choose a reason for hiding this comment

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

인라인 스타일 문제점 동일 !

<BodyText
sendMessage={sendMessage}
onTextChange={setChatMsgLength}
maxChatMsgLength={maxChatMsgLength}
/>
</div>
</div>
),
image: (
<BodyImage
uploadedImage={uploadedImage as File}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const MessageText = ({ text, color }: MessageTextProps) => (
color,
wordBreak: "break-all",
...theme.font14,
whiteSpace: "pre-line",
whiteSpace: "break-spaces",
}}
className="selectable"
>
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/hooks/chat/useSendMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default (
if (["text", "account"].includes(type)) {
// 메시지가 정규식 검사에서 통과하지 못했다면 전송을 막습니다.
if (!text) throw new Error();
if (type === "text" && !regExpTest.chatMsg(text)) throw new Error();
if (type === "text" && !regExpTest.chatMsg(text) && !regExpTest.chatMsgLength(text)) throw new Error();
if (type === "account" && !regExpTest.account(text))
throw new Error();

Expand Down
4 changes: 3 additions & 1 deletion packages/web/src/tools/regExpTest.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// 모든 정규식들은 아래 링크의 규칙을 기반으로 함
// https://www.notion.so/sparcs/Input-value-Regular-Expression-4f3e778c3b884cfe9a1b5a733c8da8fb

const chatMsg = (x) => RegExp("^[\\s\\S]{1,500}$").test(x);
const chatMsg = (x) => RegExp("^\\s{0,}\\S{1}[\\s\\S]{0,}$").test(x);
const chatMsgLength = (x) => RegExp("^[\\s\\S]{1,140}$").test(x);
Comment on lines +4 to +5
Copy link

Choose a reason for hiding this comment

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

@jinhyeonkwon 흠... ^\\S{1}[\\s\\S]{0,139}$
const chatMsg = (x) => RegExp("^\\s{0,}\\S{1}[\\s\\S]{0,}$").test(x) && RegExp("^[\\s\\S]{1,140}$").test(x);

로 통합할 수 있지 않을까요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 @xMHW 님의 확인 부탁드리겠습니다..!

Copy link

Choose a reason for hiding this comment

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

통합해도 상관없습니다~ 그냥 명시적으로 역할이 달라서 구분해놓았습니다. 위에 regex는 빈칸만 있는 것 제외하는 용도이고, 아래는 글자수제한입니다.


const nickname = (x) => {
return RegExp("^[A-Za-z가-힣ㄱ-ㅎㅏ-ㅣ0-9-_ ]{3,25}$").test(x);
Expand Down Expand Up @@ -31,6 +32,7 @@ const account = (x) => {

export default {
chatMsg,
chatMsgLength,
nickname,
phoneNumber,
roomName,
Expand Down
11 changes: 11 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.