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

fix: remove emoji #393

Merged
merged 4 commits into from
Sep 29, 2023
Merged

fix: remove emoji #393

merged 4 commits into from
Sep 29, 2023

Conversation

oganessone718
Copy link
Contributor

fixes #232
일단 이모티콘 버튼 없애봤습니다...! 문제 있다면 알려주세요.

@oganessone718 oganessone718 self-assigned this Sep 21, 2023
@netlify
Copy link

netlify bot commented Sep 21, 2023

Deploy Preview for biseo-preview ready!

Name Link
🔨 Latest commit 82dcc8a
🔍 Latest deploy log https://app.netlify.com/sites/biseo-preview/deploys/6513fc214a79d600084172c2
😎 Deploy Preview https://deploy-preview-393--biseo-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

Choose a reason for hiding this comment

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

엇 혹시 npm install 로 패키지 설치하셨나요? 비서에서는 pnpm 쓰고있어서 npm 대신 pnpm 사용해야 해요!
이 파일은 지워주시면 될 것 같습니당

@@ -1,7 +1,7 @@
import React, { useCallback, useMemo } from "react";
import { css } from "@emotion/react";

import { EmoticonIcon, SendIcon } from "@/assets";
// import { EmoticonIcon, SendIcon } from "@/assets";
Copy link
Member

Choose a reason for hiding this comment

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

코드 삭제는 어차피 기록에 남기 때문에 굳이 주석 처리하고 남겨두는 것보다는 아예 지워 주시면 될 것 같아요!

Suggested change
// import { EmoticonIcon, SendIcon } from "@/assets";


import { EmoticonIcon, SendIcon } from "@/assets";
// import { EmoticonIcon, SendIcon } from "@/assets";
import { SendIcon } from "@/assets"; // remove emoji
Copy link
Member

Choose a reason for hiding this comment

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

코드에 대해서 설명을 남기고 싶을 때는 코드 내에 주석으로 남기는 것보다 깃헙 PR 내에서 자신의 작업사항에 대해서도 댓글을 남길 수 있으니 그렇게 남겨주시면 될 것 같아요!

Suggested change
import { SendIcon } from "@/assets"; // remove emoji
import { SendIcon } from "@/assets";

@@ -86,7 +86,7 @@ export const ChatInput: React.FC<Props> = ({ send }) => {
/>
<Divider dir="vertical" />
</div>
<EmoticonIcon />
{/* <EmoticonIcon /> */}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{/* <EmoticonIcon /> */}

withSang
withSang previously approved these changes Sep 24, 2023
Copy link
Member

@withSang withSang left a comment

Choose a reason for hiding this comment

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

좋은 것 같습니다! 감사합니다.

@@ -86,7 +86,7 @@ export const ChatInput: React.FC<Props> = ({ send }) => {
/>
<Divider dir="vertical" />
</div>
<EmoticonIcon />
{/* <EmoticonIcon /> */}
Copy link
Member

Choose a reason for hiding this comment

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

아래처럼 {/* TODO: Add EmoticonIcon */} 을 추가해주셔도 좋습니다!

Copy link
Contributor

@babycroc babycroc left a comment

Choose a reason for hiding this comment

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

순호랑 상이 코멘트만 반영해주면 좋을 것 같아용! 고생했습니다아

Copy link
Member

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.

삭제한줄 알았는데 실수로 또 그냥 올렸네요 ㅜㅜ 다시push 했어용...

Copy link
Contributor

Choose a reason for hiding this comment

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

이거 없애려면 git rm 커맨드 이용해야해요 그래야 깃허브에서도 지워질거같습니다.

@@ -86,7 +85,7 @@ export const ChatInput: React.FC<Props> = ({ send }) => {
/>
<Divider dir="vertical" />
</div>
<EmoticonIcon />
{/* TODO: Add EmoticonIcon */}
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@SnowSuno SnowSuno left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@withSang withSang left a comment

Choose a reason for hiding this comment

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

LGTV! 🖥

@oganessone718 oganessone718 merged commit b89f9b6 into main Sep 29, 2023
@oganessone718 oganessone718 deleted the remove-emoji branch September 29, 2023 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[채팅] 이모티콘 버튼 미동작
5 participants