Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: 새 구인글 생성 페이지 & 산출물 등록 페이지 UI 레이아웃 및 기능 구현 #27

Merged
merged 59 commits into from
Jan 10, 2024

Conversation

prgmr99
Copy link
Member

@prgmr99 prgmr99 commented Jan 9, 2024

구현 내용/방법

간단하게 구현한 내용과 방법에 대한 설명

  • 새 구인글 생성 & 산출물 등록 페이지 UI 레이아웃
  • 필수기능 구현
  • 유효성 검사

리뷰 필요

나중에 다시 고민해야할 내용이 있는 내용

없을 경우 작성 X

있을 경우 작성 후 이슈 남기고 해당 PR 링크

  • 수연님,,,코드가 많기는 한데, 다 재활용한 코드라서 크게 보실거는 없을거에요!...아마도;; 🧐

close #26

prgmr99 added 30 commits January 2, 2024 14:05
member: false,
inform: false,
});

const goHome = () => {
navigate('/');
Copy link
Contributor

Choose a reason for hiding this comment

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

유저 밋팀 관리 페이지의 사이드바는 NavLink 사용하셨지만, 헤더는 useNavigate 로 사용하신 이유가 궁금합니다!🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

NavLink를 사용하면 state을 사용하지 않게 됩니다!!
react-router-dom에서 NavLink를 사용할때, active라는 props를 자체적으로 포함하고 있어서 활성화 여부에 따라 특정 css 효과를 줄 수 있게됩니다!

NavLink를 사용하지 않으면 state을 만들고, 예를 들어서 눌러서 활성화됬는지 상태를 관리해줘야 합니다!!

저도 자세히는 몰랐는데 덕분에 알게 되었네요!! 나중에 통일시켜야겠습니다..ㅎ

Copy link
Member Author

Choose a reason for hiding this comment

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

근데 NavLink를 까봐야 알겠지만 라이브러리 자체에서 state을 사용하도록 구현했을 가능성이 높을 것 같아서
어떤 것을 사용해도 큰 문제는 없을 것 같습니다..!

Copy link
Contributor

Choose a reason for hiding this comment

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

아하! NavLink 는 style을 부가적으로 줄 수 있군요!

근데 useNavigate 도 헤더에서 스테이트 없이 사용하지시 않으셨나요??🤔
사실 스타일을 제외하고 NavLink 와의 성능 차이? 가 있는지가 궁금해서 여쭤봤습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

isHere 이라는 state을 줬습니다!!
성능차이를 신경쓸만큼 영향이 있지는 않습니다..!!

<S.Dropdown allowNeed={allowNeed}>
<div className='menu' onClick={onClickDropdown} ref={dropdownRef}>
<label>
{currentValue} {showDropdown ? '▲' : '▼'}
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 icon이 알고보니 피그마에서 svg로 되어있네요!☺️

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋ심사숙고해서 고른 아이콘이지만, svg로 변경하는게 훨씬 이뻐보일 것 같네요!
수정해보겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

오.. 드롭다운 따로 분리해주셨군요! utils 로 넣어도 좋을 듯 합니다!!

Copy link
Member Author

@prgmr99 prgmr99 Jan 10, 2024

Choose a reason for hiding this comment

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

utils에 넣어야할까요???
components에 존재하는게 맞을 것 같은데, utils에는 유효성 검사 같이 컴포넌트와 관계없이 사용될 수 있는 파일(쉽게 설명하면 css가 없는 로직같은 파일)들이 들어가는게 좋다고 생각해요!!

물론, 정답은 없지만 이 블로그 한 번 참고해주셔도 좋을 것 같아요!
https://time-map-installer.tistory.com/196

Copy link
Contributor

@kimsuyeon0916 kimsuyeon0916 Jan 10, 2024

Choose a reason for hiding this comment

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

제가 폴더의 사용성에 대해 잘못 인지하고 있던 부분이 있네요! 저는 유틸 컴포넌트도 같이 다룬다고 생각했었습니다!
css 가 없는 로직이라고 하시니까 확 와닿네요!

그럼 hooks 든 utils 든 css 가 없는 로직들을 포함하는 거군요..!!!!!

알려주셔서 감사합니다!😊

Copy link
Member Author

Choose a reason for hiding this comment

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

아니에요!!! 제가 더 감사합니다 :)

Comment on lines +91 to +99
user-select: none;
-webkit-user-select: none;
/* (safari, chrome) browsers */
-moz-user-select: none;
/* mozilla browsers */
-khtml-user-select: none;
/* konqueror browsers */

-ms-user-select: none; /* IE10+ */
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분을 추가하신 이유가 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

이 부분은 저희가 웹사이트에서 더블 클릭을 하면 파란색 하이라이트(선택)이 뜨는 것을 없애주는 코드입니다!

Copy link
Contributor

@kimsuyeon0916 kimsuyeon0916 Jan 10, 2024

Choose a reason for hiding this comment

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

아하! 승준님 덕분에 알게 됐습니다!! 알려주셔서 감사합니다!!

그럼 없애시려는 의도도 여쭤볼 수 있을까요??
궁금합니당!🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

드롭다운 계속 빠르게 클릭하다가 뜨는거보고 좀 별로여서 삭제했어요ㅋㅋ

@@ -1,7 +1,7 @@
import { atom } from 'recoil';

export const areaState = atom({
key: 'areaState',
key: 'areaState1',
Copy link
Contributor

Choose a reason for hiding this comment

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

1을 붙이신 이유가 궁금합니다!🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

저 부분은 warning이 떠서 추가했습니다!

warning을 나중에 읽어보시면 무시해도 좋다고 나와있는데 그냥 1을 추가해서 warning을 없앴습니다!

아래의 블로그는 해당 warning에 대해서 설명해줘요!

https://dogcoder.tistory.com/entry/%EC%9D%B4%EC%8A%88-Nextjs-Recoil-duplicate-atom-key-%EC%98%A4%EB%A5%98-%ED%95%B4%EA%B2%B0%ED%95%98%EA%B8%B0

Copy link
Contributor

Choose a reason for hiding this comment

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

말씀해주신 블로그 참고하니까 이해가 됐어요!😊 알려주셔서 감사해요ㅜㅜ

그럼 _Recoil에서 제공하는 RecoilEnv를 통해 환경변수 별도로 설정_하는 방법으로 하시는 건 어떠신가요 승준님??🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 생각이네요!
자세히 알아봐야겠네요!!

src/main.tsx Outdated
@@ -42,10 +48,36 @@ const router = createBrowserRouter([
path: 'create', // meeteam/create
Copy link
Contributor

Choose a reason for hiding this comment

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

밋팀 생성은 create/meeteam 가 아닌 create로 하신 이유가 궁금합니다!

또한 주석 처리 하신 부분을 고민하고 계신 건가용??

Copy link
Member Author

Choose a reason for hiding this comment

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

우선은 create/meeteam로 하는게 맞겠네요!
저 부분은 일단 처음에 create으로 해서 아직까지 변경하지 않은 부분이어서 그렇습니다..!!

주석처리한 부분은 고민하고 있었던 부분이 있는데, 그건 아닌 것 같네요! 수정해놓겠습니다!!

import React from 'react';
import S from './Status.styled';

interface IStatus {
Copy link
Contributor

Choose a reason for hiding this comment

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

승준님은 인터페이스 네이밍을 이렇게 하시는 편이신가요?? 같이 컨벤션 정해보면 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

인터페이스 네이밍 컨벤션이기는 합니다! 보통 I + 인터페이스를 적용할 요소 이런식으로 사용합니다!
절대적인 것은 아니고 보통 이렇게 사용해서 저도 저렇게 사용했습니다!!
더 편하신 것이 있을까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/basarat/typescript-book/blob/master/docs/styleguide/styleguide.md#interface

https://geekk89.medium.com/%EA%B0%9C%EB%B0%9C%EC%97%90%EC%84%9C%EC%9D%98-%ED%95%84%EC%9A%94%ED%95%9C-%EC%BB%A8%EB%B2%A4%EC%85%98-7d6ce8f2332c

위에 참고 링크 달았습니다!

저는 (컴포넌트)Props 라고 뒤에 붙이는 걸 선호하긴 합니다만,
더 좋은 컨벤션이 있으면 같이 통일하고 싶어요!

Copy link
Member Author

@prgmr99 prgmr99 Jan 11, 2024

Choose a reason for hiding this comment

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

오호! I를 사용하는 거는 지양하는 방식이었군요!
저도 추가적으로 찾아보니까 그냥 특별하게 붙이지 않는 것이 가장 좋은 것 같고, 실제로 가장 많이 쓰는 방법인 것 같네요!

마이크로소프트 types 지정 파일
https://github.com/microsoft/TypeScript/blob/main/src/services/types.ts

다른 글들이 어려웠는데 아래에 글은 잘 정리해주신 것 같아요!
https://yogjin-dev.vercel.app/posts/Do-not-use-Hungarian-notation

어려운 글..
https://toss.tech/article/typescript-type-compatibility
구조적 타이핑이라는 개념을 이참에 알아보시면 좋을 것 같아요!! (저는 처음 알았어요ㅋㅋ;; 아님 잊고 있었나봐여;;)

참고하면 좋은 글
https://techblog.woowahan.com/9804/#toc-3

수연님 덕분에 잘못된 점을 바로 잡을 수 있었네요! 그리고 더 깊은 내용까지 알 수 있었습니다!
감사합니다🫡🫡

(앗 참고로 Props는 좀 걱정되는게 저희 인터페이스를 props에만 사용하는게 아니어서 props가 좀 맞지 않을 것 같다는 생각이 들었습니다..)

})}
<input
type='text'
placeholder='태그를 입력하고 엔터를 누르세요.'
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 플레이스 홀더가 입력을 해도 남아있는 걸 의도하신 건가요?🤔

Copy link
Member Author

@prgmr99 prgmr99 Jan 10, 2024

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.

별로 아닙니다! 살짝 취향 차이 것 같네요!

내일 개발회의때 백엔드 분들한테도 여쭤보면 좋을 듯 합니다!😄

})}
</div>
)}
{isDropdownVisible && (
Copy link
Contributor

Choose a reason for hiding this comment

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

승준님 혹시 해당 태그 드롭다운이 어떨 때 열리시는 걸 의도하신 건지 궁금합니다!🤔

Copy link
Member 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.

저는 입력 커서가 활성화 되어있을 때 보이는게 자연스럽다라고 생각하긴 했는데,
내일 개발회의에서 다른 팀원분들한테도 여쭤보면 좋을 것 같습니다!

그리고 추가적으로는

image

태그가 2줄 이상일 때는 가려져서 레이아웃을 살짝 수정하셔야 될 것 같습니다!🥺

Copy link
Member Author

Choose a reason for hiding this comment

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

아하..!! 수정해놓을게요!!!
이런 테스트 너무 소중하네요ㅠㅠ 감사해요👍👍👍

Comment on lines +211 to +222
<div className='container__img-input'>
<input
type='file'
accept='image/*'
id='meeteamImg'
placeholder='이미지를 업로드해주세요.'
onChange={onChangeImg}
/>
<label className={file ? 'haveFile' : ''} htmlFor='meeteamImg'>
{file ? `${fileName}` : '이미지를 업로드해주세요.'}
</label>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 부분에 이미지 미리보기는 따로 들어가진 않나요?!!?🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

이거는 디자인도 그렇고 따로 얘기된 부분도 없어서 일단은 어떻게 될지 몰라서 안 넣었습니다!

Comment on lines +76 to +82
const onChangeImg = useCallback((event: React.ChangeEvent<HTMLInputElement>) => {
if (event.target.files !== null) {
setFileName(event.target.files[0].name);
const selectedFiles = event.target.files as FileList;
setFile(URL.createObjectURL(selectedFiles?.[0]));
}
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

이미지가 1개밖에 등록이 안되서 나중에 다중 파일을 저장할 수 있게 수정해야 할 것 같습니다!👍🏻

Copy link
Member 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.

그렇네요!!! 밋팀 생성할 때의 대표 이미지만 필요 할 것 같습니다!

산출물만 저번에 같이 논의한 다중 이미지 저장과 미리보기 뷰만 따로 추가하면 될 것 같습니다!

@prgmr99 prgmr99 merged commit f3be8f8 into develop Jan 10, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Priority: High 우선순위: 높음 Status: Available 상태: 작업을 바로 시작할 수 있는 상태 Status: Review Needed 상태: 리뷰 대기중인 상태 Type: Feature 유형: 기능 추가 Type: Style 유형: 스타일
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: 새 구인글 생성 페이지 & 산출물 등록 페이지 UI 레이아웃 및 기능 구현
2 participants