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

[FE] 피드백 미작성시 받은 피드백 안보이는 기능 추가(#703) #706

Merged
merged 26 commits into from
Nov 4, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 3, 2024

📓 스토리북 링크

바로가기

📌 관련 이슈

✨ PR 세부 내용

  • 상대방 피드백 작성 안 했을 때 블러처리
    🔥 블러처리만 했을 때는 뒤에 textarea가 클릭이 돼서 투명도 50%인 div 태그(Overlay)로 위를 덮어서 클릭을 막았습니다.
    image

  • 피드백 작성 모달에서 페이지로 변경
    🔥 피드백 작성 폼으로 바로 이동하게 하기 위해 모달에서 페이지로 변경하고 그 페이지로 리다이렉트 했습니다.
    image

  • feedbackLayout 컴포넌트에서 useFeedbackForm이라는 커스텀 훅이 사용되었는데 리팩토링 과정에서 가독성을 위해 제거하였습니다. 추후에 커스텀 훅으로 분리해야할 필요성을 느끼면 리팩토링하겠습니다!

  • UI/UX 대해서 공부를 하다가 알게된 사실이 유효한 폼을 작성하지 않았을 때 버튼을 disabled을 하는 것보다 버튼은 항상 활성화 되어있고, 제출 버튼을 눌렀을 때 유효하지 않은 필드에 대해 에러를 띄워주는 것이 더 좋다는 것이었습니다. 이에 따라 코드를 변경하였습니다.

@github-actions github-actions bot added FE 프론트 개발 관련 작업 기능 기능 구현 작업 labels Nov 3, 2024
Copy link
Member

@00kang 00kang left a comment

Choose a reason for hiding this comment

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

텐텐 작업하느라 수고하셨어요!! 👍👍😂
파일 체인지 어마무시하네요,,

몇가지 질문이랑 제안사항 남겨보았으니, 확인해주세요!

1. 피드백 작성/수정 페이지의 디자인 수정 제안

현재는 방의 정보와 피드백 관련 문항들이 잘 구분되고 있지 않은 것 같아요! 이렇게 구분하는 디자인은 어떤가요?
아래 ReviewerFeedbackLayout.tsxReviewerFeedbackLayout.style.ts 에 제안 코드를 작성해 두었으니 반영할 때 참고하시면 될 것 같아요~ (RevieweeFeedback도 같이!)

+) 현재 피드백 작성/수정 페이지에서 폼이 헤더네비게이션바 쪽으로 붙어 있던데 가운데 정렬해도 좋을 것 같아용

before after

2. 오류 발견

모바일 반응형일 때만 나타나는 햄버거 메뉴에서 검정색으로 보이고 있는데, 버튼 속성을 수정하면서 이렇게 된 것 같아요.
확인 부탁드려요!

rows={10}
maxLength={2000}
showCharCount={true}
value={feedbackCardData.feedbackText.length ? feedbackCardData.feedbackText : "없음"}
Copy link
Member

Choose a reason for hiding this comment

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

FeedbackCard에서는 없음이고, ReviewerFeedbackForm과 RevieweeFeedbackForm은 미작성으로 다르게 나타내는 이유가 있나요?

전부 미작성이라는 표현으로 통일시켜도 좋을 것 같아요!

Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 그냥 놓치고 넘어간 것 같아요! 미작성으로 통일 시키겠습니다~~

Comment on lines +31 to +46
const variantStyles = (variant: ButtonVariant, outline?: boolean) => {
return css`
${outline
? css`
color: ${({ theme }) => getColor(theme, variant)};
background-color: ${({ theme }) => theme.COLOR.white};
border: 2px solid ${({ theme }) => getColor(theme, variant)};
`
: css`
color: ${({ theme }) =>
variant === "default" || variant === "confirm" ? theme.COLOR.black : theme.COLOR.white};
background-color: ${({ theme }) => getColor(theme, variant)};
border: 2px solid ${({ theme }) => getColor(theme, variant)};
`}
`;
};
Copy link
Member

Choose a reason for hiding this comment

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

outline이 true일 때: 배경색은 흰색, 테두리와 텍스트는 variant 색상
outline이 false일 때: 배경색과 테두리는 variant 색상, 텍스트는 흰색/검정(배경이 검정, 초록일 때만)

이렇게 구분해 놓으니 더 유연한 컴포넌트가 된 것 같아 좋네요!

options={getDevelopKeywordOptions(formState.evaluationPoint)}
/>
</S.ItemContainer>

<S.ItemContainer>
<S.QuestionContainer>
<S.ModalQuestion>리뷰이의 코드를 추천하시나요? (비공개 항목)</S.ModalQuestion>
<S.ModalQuestion>[비공개] 리뷰이의 코드를 추천하시나요?</S.ModalQuestion>
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

@chlwlstlf chlwlstlf Nov 4, 2024

Choose a reason for hiding this comment

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

QA 받았던 부분인데 [비공개] 문구 어떤지 얘기 나눠 보려고 저렇게 해두었습니다!
괜찮은 것 같다면 다른 부분에도 [공개]로 변경해둘게요ㅎㅎ

❓ 리뷰어 피드백 부분은 다 공개인데 거기도 [공개]를 추가할까요??

Copy link
Member

Choose a reason for hiding this comment

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

피드백 폼의 통일성을 위해서 해도 좋을 것 같아요!

Comment on lines 89 to 90
feedbackPageType === "view"
? "없음"
Copy link
Member

Choose a reason for hiding this comment

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

ReviewerFeedbackForm과 동일하게 미작성으로 나타내는 거 어떨까요?

Comment on lines 112 to 117
<S.PageType>
<>{reviewee?.username} </>
{feedbackPageType === "create" && "리뷰이 피드백 작성하기"}
{feedbackPageType === "edit" && "리뷰이 피드백 수정하기"}
{feedbackPageType === "view" && "리뷰이 피드백 확인하기"}
</S.PageType>
Copy link
Member

Choose a reason for hiding this comment

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

현재 ~~~ 리뷰이 피드백 작성하기 이렇게 보여지고 있는데, 리뷰이(~~~) 피드백 작성하기 이렇게 수정하는 건 어떤가요?
image

Copy link
Contributor

Choose a reason for hiding this comment

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

오 괜찮은 것 같아요! 문구 변경하겠습니다😊

Comment on lines 22 to 24
export const ModalQuestion = styled.legend<{ $isInvalid?: boolean }>`
font: ${({ theme }) => theme.TEXT.small_bold};
color: ${({ theme }) => theme.COLOR.grey4};
color: ${({ theme, $isInvalid }) => ($isInvalid ? theme.COLOR.error : theme.COLOR.grey4)};
Copy link
Member

Choose a reason for hiding this comment

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

여기에 line-height: normal 속성 추가해도 좋을 것 같아요!
image

Copy link
Contributor

Choose a reason for hiding this comment

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

변경했습니다! 추가로 아래처럼 필수입력이 옆으로 가게 변경하였습니다😁
image

Comment on lines 114 to 129
<S.PageTitle>{roomInfo.title}</S.PageTitle>
<S.Keywords>
{displayedKeywords.length === 0 ? (
<S.NoKeywordText>지정된 키워드 없음</S.NoKeywordText>
) : (
displayedKeywords.map((keyword) => (
<Label
key={keyword}
type="KEYWORD"
text={keyword}
size="semiSmall"
backgroundColor={theme.COLOR.grey0}
/>
))
)}
</S.Keywords>
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
<S.PageTitle>{roomInfo.title}</S.PageTitle>
<S.Keywords>
{displayedKeywords.length === 0 ? (
<S.NoKeywordText>지정된 키워드 없음</S.NoKeywordText>
) : (
displayedKeywords.map((keyword) => (
<Label
key={keyword}
type="KEYWORD"
text={keyword}
size="semiSmall"
backgroundColor={theme.COLOR.grey0}
/>
))
)}
</S.Keywords>
<S.MissionInfoContainer>
<S.PageTitle>{roomInfo.title}</S.PageTitle>
<S.Keywords>
{displayedKeywords.length === 0 ? (
<S.NoKeywordText>지정된 키워드 없음</S.NoKeywordText>
) : (
displayedKeywords.map((keyword) => (
<Label
key={keyword}
type="KEYWORD"
text={keyword}
size="semiSmall"
backgroundColor={theme.COLOR.grey0}
/>
))
)}
</S.Keywords>
</S.MissionInfoContainer>

@@ -16,12 +17,12 @@ export const FeedbackContainerHeader = styled.div`
gap: 1.2rem;
`;

export const ModalType = styled.p`
export const PageType = styled.p`
font: ${({ theme }) => theme.TEXT.small_bold};
color: ${({ theme }) => theme.COLOR.grey3};
`;

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
export const MissionInfoContainer = styled.div`
display: flex;
flex-direction: column;
gap: 1rem;
padding: 1rem;
border: 1px solid ${({ theme }) => theme.COLOR.grey1};
border-radius: 8px;
`;

Copy link
Contributor

Choose a reason for hiding this comment

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

코드 포함해서 제안 주셔서 감사합니다😭
이렇게 하면 방에 관한 정보는 box 안에 묶여 있어 사용자가 더 파악하기 쉽겠네요!

Copy link
Contributor

@pp449 pp449 left a comment

Choose a reason for hiding this comment

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

파일체인지를 보니 엄청난 작업량이 느껴지네요,,

크게 남길만한 코멘트는 없지만

https://github.com/user-attachments/assets/b1af761a-b0e0-4e85-a43d-e915ebd33eee
여기서 선택하지 않으면 문항 색깔이 바뀌는데 색깔 바뀌는게 아닌 해당 문항을 필수로 체크해야해요 같은 문구를 띄어주는건 어떨까요

Comment on lines +53 to +57
useEffect(() => {
if (feedbackData) {
setFormState(getInitialFormState(reviewer, feedbackData));
}
}, [reviewer, feedbackData]);
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 페이지는 연속적으로 렌더링이 아닌
언마운트 후 다시 마운트가 되는것으로 예상이 되는데 해당 useEffect 코드가 없으면 에러가 발생하나요??

Copy link
Contributor

Choose a reason for hiding this comment

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

방 생성 입력 폼에서는 useEffect가 없어도 데이터를 잘 불러와서 여기도 없애 보았는데요.. 빈 값으로 보입니다.. api로 값은 잘 받아와져요 그냥 화면에 안 뜹니다.

이 컴포넌트에서 뎁쓰가 깊게는 2번을 더 들어가게 되는데요. 이 부분에 값이 안 들어가는 것으로 생각하고 있어요! 다음 PR에서 자세히 확인해본 후 useEffect 코드를 제거할 수 있는 방안을 생각해보겠습니다😁

@00kang
Copy link
Member

00kang commented Nov 4, 2024

작업 확인 완료~

@pp449 pp449 merged commit a27546a into develop Nov 4, 2024
3 checks passed
@pp449 pp449 deleted the feat/#703 branch November 4, 2024 14:01
@chlwlstlf
Copy link
Contributor

여기서 선택하지 않으면 문항 색깔이 바뀌는데 색깔 바뀌는게 아닌 해당 문항을 필수로 체크해야해요 같은 문구를 띄어주는건 어떨까요

저도 좋다고 생각합니다!! 입력 페이지에서 통일성 있는 UI를 아직 팀 내부에서 생각해보지 않았는데 같이 이야기 해보면 좋을 것 같아요ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트 개발 관련 작업 기능 기능 구현 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] 피드백 미작성시 받은 피드백 안보이는 기능 추가
3 participants