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

[3주차] 최애 카공장소 고르기 월드컵 #3

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

choiyoorim
Copy link
Collaborator

✨ 구현 기능 명세

카공하기에 좋은 카페를 고를 수 있는 월드컵 기능

  • 선택한 후보들을 모아 재투표, 이후 결선을 통해 가장 좋은 카페를 선정 가능

🎁 PR Point

  • useEffect를 활용하지 못했는데, 어떻게 활용해야 좋은건지 궁금합니다.
  • 전체적인 로직이 깔끔하지는 못한데, 다른 방식도 궁금합니다. 특히 두가지 후보군을 보여줄 때 display라는 배열에 넣어서 이를 map으로 돌려서 출력해주고 있는데, 좋은 방법이 아닌 것 같아서... 여러분들의 방법이 궁금합니다.

😭 어려웠던 점

  • styled component를 사용하는 방법을 좀 더 공부해야할 것 같습니다. 특히 css 디자인을 styled component를 통해서 더 많이 변경해보고 싶었는데 시간상 다 못한게 아쉬워서 마저 도전해보려고 합니다. styled component를 이해하기는 했는데 좀 더 다양한 사용 예시를 보면서 공부해야할 것 같습니다.
  • useEffect에 대한 잘못된 이해를 가지고 있었습니다. 그래서 어떤 순서로 화면에 출력되는지를 헷갈려서 오류를 많이 겪었는데, 정확히 화면이 출력되는 순서를 이해하는 것이 가장 중요할 것 같습니다.

😎 구현 결과물

스크린샷 2022-05-06 오전 2 51 46

@choiyoorim choiyoorim added the 3️⃣ 3주차 3주차 과제에요. label May 5, 2022
@choiyoorim choiyoorim requested review from aeuna and NamJwong May 5, 2022 17:59
@choiyoorim choiyoorim self-assigned this May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

오늘도 할할놀놀을 몸소 실천 중인 웹파트원 ! 화이팅 :)
과제 레퍼런스 참고해서 빠트린 부분은 없는 지 체크해볼까요?

Copy link

@aeuna aeuna left a comment

Choose a reason for hiding this comment

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

  • useEffect 활용하는 부분은 저도 처음에 어려웠어요 ㅠㅠ 아래 첨부한 링크를 정말 여러번 읽었답니다 ㅠ
    로직생각하면서, 크게 컴포넌트가 화면에 처음에 나타날때 or 어떤 props가 바뀔 때 or 컴포넌트가 화면에서 사라질때 이 3가지에서 어떤 동작을 시켜주고 싶은지 생각하면 조금이나마 도움이 될 것 같습니다!
  • 저도 보여주고 자 하는 정보2개를 display라는 state로 관리 해주고 있습니다! 이 부분은 다른 분들의 코드도 참고해보면 좋을 것 같아요!!
    3주차 과제도 수고하셨어요 :)

week3/src/App.js Outdated
margin:50px;
`

const Crown = styled.div`
Copy link

Choose a reason for hiding this comment

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

Crown 이랑 Vs styled-components에서 position, left, right가 겹치는 것 같아요!
컴포넌트를 확장해서 표현을 할수도 있을것 같아요

const IconWrapper = styled.div`
	position : fixed;
	left : 50px;
	right : 50px;
`;

const Crown = styled(IconWrapper)`
	font-size : 100px;
	bottom: 400px;
`;

const Vs = styled(IconWrapper)`
	font-size : 50px;
	bottom : 370px;
`;

아니면 두개의 컴포넌트가 다른게 font-size와 bottom 밖에 없으니까 styled-components에 props를 전달해줘서 판단해주는 방법으로 해도 괜찮을 것 같아용!

week3/src/App.js Outdated

`

function shuffleArray(){
Copy link

Choose a reason for hiding this comment

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

shuffle함수를 따로 뺀 것 좋은 것 같아요!
혹시 App 상단에 shuffleArray 함수와 winner배열을 위치 시켜 놓은 이유가 있나요? 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

shuffleArray는 특별한 이유는 없고, 아마 작성한 순서대로 그대로 올려둬서 그런 것 같습니다
winner배열은 전역변수처럼 써야하는 이슈가 생겨서 일부러 가장 위에 배치시키면서 전역변수로 사용하였습니다.
winner배열의 길이에 따라 분기문을 처리해야하는 부분에서 실행순서가 완전히 보장받지 못했다보니까 전역변수로 활용한... 아주 안좋은 코드임을 이번주 세미나 들으며 깨달았내요 ㅎ핳

week3/src/App.js Outdated
const [coffeeShop, setCoffeeShop] = useState(items);
let display = [];

useEffect(()=>{
Copy link

Choose a reason for hiding this comment

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

제가 알기론 useEffect 에서 의존성 배열안에 특정값을 넣어주게 되면,
컴포넌트가 처음 마운트 될 때에도 호출이 되고, 지정한 값이 바뀔 때에도 호출이 되고, return해서 함수까지 반환하게 된다면 배열 안에 특정 값이 있다면 언마운트시에도 호출이되고, 값이 바뀌기 직전에도 호출이 된다고 해여!
이 부분 유의하셔서 아래 77라인에서 79라인 코드도 useEffect 안에서 처리해줄 수 있을 것 같아요!
저도 처음에 이부분이 많이 헷갈렸는데 도움이 되었던 자료 첨부드릴게용
https://react.vlpt.us/basic/16-useEffect.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 링크 한번 참고해서 또 공부해보겠습니다. 제가 여러번 풀어쓴 코드를 useEffect 이내에서 모두 처리할 수 있다는 말씀이군요...
이것도 한번 수정해보겠습니다 ㅎㅎㅎ

week3/src/App.js Outdated
let winner = [];
function App() {
const [coffeeShop, setCoffeeShop] = useState(items);
let display = [];
Copy link

Choose a reason for hiding this comment

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

display와 winner 를 state로 처리하면 좋을 것 같아요!
컴포넌트에서 사용하는 상태(state)가 변경되면, 컴포넌트는 리 렌더링되면서 변하는 값을 표시를 해준다고 해요!
state로 관리하지 않게 되면 실질적으로 winner나 display가 변했는데 리 렌더링되지 않아서 화면에서는 표시되지 않는 현상이 발생하지 않을까.. 라는 생각이 들어요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앗 제가 왜 display와 winner를 따로 state로 설정해주지 않았냐면.. 제가 지금 state로 설정한 부분이 coffeeShop부분이니까 이 부분이 한번 고쳐지면 렌더링이 돌아가면서 display와 winner가 새롭게 바뀐 채 출력이 될 것이라고 예상을 했었던 거였어요! 그래서 모두를 state로 처리하게 되면... 여러번 같은 화면이 렌더링될 것이라는 예상이었는데 제가 잘못 알고 있는 부분이 있으까요...?!

week3/src/App.js Outdated
<div className="item" key={coffeeShop[0].name}/>
<Img className="coffee-img" src={coffeeShop[0].pic} alt="커피"></Img>
<Crown>👑</Crown>
<div className="coffee-name">{coffeeShop[0].name}</div>
Copy link

Choose a reason for hiding this comment

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

App.css에서 따로 coffee-name을 지정해주고 있는데, 위에서 styled-components를 사용하고 있으니 일관성 있게 이 부분도 styled-components 로 작성해주면 좋을 것 같아요!
그리고 styled-components를 사용해서 스타일을 적용 시켜주고 있으니, 따로 class 이용하지 않는다면 className을 생략해도 되지 않을 까 싶습니다!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 좋은 아이디어 감사합니당! styled-components가 익숙치 않아서 이러한 스타일 변경은 어떡해야하지 싶어서 classname을 작성하고 css로 고쳐주느라 classname을 작성해준 듯 싶어요@.@ 한번 이 부분도 고쳐나가보겠슴다 ㅎㅎ

@github-actions
Copy link

github-actions bot commented Jul 5, 2022

오늘도 할할놀놀을 몸소 실천 중인 웹파트원 ! 화이팅 :)
과제 레퍼런스 참고해서 빠트린 부분은 없는 지 체크해볼까요?

Copy link

@Nahee-Park Nahee-Park left a comment

Choose a reason for hiding this comment

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

고생했어여! 타스 정복 갈겨 !

값이 바뀔 수 있는 변수를 선언할 때 어떤 방식으로 선언하는지에 대한 이유가 확실하면 좋을 것 같아여 !
state를 쓸 수도 있고 ref를 쓸 수도 있고 let을 사용할 수도 있는데 왜 그걸 쓰는 지에 대한 이유가 있으면 좋을 것 같습니다 !

궁금한 부분들 질문 갈겼으니까 답변해주세여 헤헤

Comment on lines +72 to +77
interface coffeeShop {
name: string,
pic: string,
}
function App() {
const [coffeeShop, setCoffeeShop] = useState(items);

Choose a reason for hiding this comment

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

type이나 interface의 이름은 컨벤션이나 클린코드의 영역이지만, state랑 interface의 이름을 같게 하는 것은 구분이 안돼서 헷갈릴 수 있을 것 같아요! 타입 이름은 다른 변수명과 겹치지 않게 해주는 것이 좋을 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 구분을 잘 해줘야겠네요 좋은 조언 고마워요!!

Comment on lines +80 to +81
useEffect(()=>{
},[coffeeShop])

Choose a reason for hiding this comment

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

이 부분의 의미는 무엇인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아마.. useEffect를 배운지 얼마 안되어서 쓴 코드 같은데 coffeeShop이라는 배열에 의해서만 렌더링 되었으면 좋겠다는 의미로 작성한 것 같네요... 지금 보니 이상하군요...?

}
function App() {
const [coffeeShop, setCoffeeShop] = useState(items);
let display:coffeeShop[] = [];

Choose a reason for hiding this comment

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

이 display 변수에서 state가 아니라 let을 사용한 이유가 무엇인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

state는 상태가 바뀌면 화면이 새롭게 렌더링되니까 변수 내용이 렌더링되지 않게끔 하려면 let으로 해줘야겠다고 생각했어요! 혹시 더 좋은 방법이 있을까요?!

Comment on lines +83 to +85
shuffleArray();
console.log(coffeeShop);
display = [coffeeShop[0],coffeeShop[1]];

Choose a reason for hiding this comment

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

이 행위들이 동작하길 원하는 시점은 어떨 때인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이건 처음의 동작이에요! 이상형월드컵이 시작되자마자 shuffle을 하고, 0과 1의 인덱스를 가진 사진들이 보여질 수 있게끔요!

console.log(coffeeShop);
display = [coffeeShop[0],coffeeShop[1]];

const clickEvent = (coffeeShops:coffeeShop) =>{

Choose a reason for hiding this comment

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

이것도 컨벤션적인 부분이지만 보통 이벤트핸들러 내부에 들어가는 함수는 handleClick / clickHandler 이런 느낌으로 이름 짓는 편이 좀 더 명시적인 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오... 확실히 더 명시적이네요!! 앞으로 참고할게요!!

<DisplayContent>
{
display.map(item => {
return <Box className="item" key={item.name} onClick={()=>clickEvent(item)}>

Choose a reason for hiding this comment

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

key 야무지게 넣었네여 ㅎㅎ 굿굿

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😍칭찬쭈은데요!!><

Comment on lines +1 to +2
declare module "*.jpeg";
declare module "*.webp";

Choose a reason for hiding this comment

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

이 모듈을 선언해준 이유는 무엇인가요 !?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이건 타입스크립트로 바꾸면서 추가한 모듈인데 기존의 자바스크립트 코드에서 사용하던 import문을 그대로 사용하려니 모듈이 없다고 인식하더라구요! 그래서 이렇게 따로 모듈을 선언해주었습니다! 이 부분은 조금 더 왜 이렇게 모듈 선언을 해주어야 하는지 공부해야할 것 같아요! 확장자가 다른 이미지를 하나로 합치는 과정인 것 같기도....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3️⃣ 3주차 3주차 과제에요.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants