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

[4주차-ts] Search BAR with TS #7

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

[4주차-ts] Search BAR with TS #7

wants to merge 16 commits into from

Conversation

henization
Copy link
Collaborator

✨ 구현 기능 명세

🎁 PR Point

😭 어려웠던 점

😎 구현 결과물

@github-actions
Copy link

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

@henization henization changed the title Week4 ts [4주차-ts] Search BAR with TS Jun 20, 2022
@henization henization self-assigned this Jun 20, 2022
@henization henization added the 4️⃣ 4주차 4주차 과제에요. label Jun 20, 2022
Copy link

@joohaem joohaem left a comment

Choose a reason for hiding this comment

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

🔥🔥🔥🔥


return (
<Styled.Root>
{/* Header에 뭐를 props로 내려주고 있나요? */}
Copy link

Choose a reason for hiding this comment

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

저에게 던지는 질문인가요>?😋😋

Comment on lines +2 to +7
id?: number;
place_url?: string;
place_name?: string;
road_address_name?: string;
distance?: string;
phone?: string;
Copy link

Choose a reason for hiding this comment

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

모두 옵셔널로 타입지정 해준 이유가 있을까요 ??

Copy link

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.

옵셔널로 안주니까 에러가 떠서 옵셔널로 줬어유!

Copy link

Choose a reason for hiding this comment

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

나니요....! 왜그러지

const GlobalStyle = createGlobalStyle`
${reset}
@font-face {
font-family: 'ParkYongJun';
Copy link

Choose a reason for hiding this comment

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

용준이형 폰트🤩

* {

box-sizing: border-box;
font-family: 'ParkYongJun';
Copy link

Choose a reason for hiding this comment

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

폰트 패밀리는 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.

근데 안돼서 새로 지정해줬어유 ... ㅈㅔ 컴의 문제일수도 있심다

Comment on lines 1 to 6
export const flexColumnCenter = `
display: flex;
flex-direction: column;
justify-contents: center;
align-items: center;
`;
Copy link

Choose a reason for hiding this comment

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

이것도 스타일링이니 아예 theme에 추가하는 게 어떨까 싶습니다!!
mixxin 폴더의 의미가 궁금해요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵!! theme에 추가하겠습니다!

Comment on lines +26 to +28
//useRef의 타입을 어떻게 정의해여!?!!?
const searchRef = useRef<HTMLInputElement>(null);
const position = useRef<Coordinates>({ longitude: 0, latitude: 0 });
Copy link

Choose a reason for hiding this comment

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

잘한 것 같은데,!! 관련 아티클 첨부합니당
useRef 의 3가지 정의와 사용법(링크)

Comment on lines 119 to 133
const Styled = {
Root: styled.header`
${flexColumnCenter}
& h1 {
margin-bottom: 2rem;
}
`,
SearchWrapper: styled.div`
display: flex;
gap: 2rem;
`,
};

const MyLocationButton = styled.button`
// 왜 색 안바뀌지
Copy link

Choose a reason for hiding this comment

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

Styled 로 묶은 SC 와 아닌 SC 의 차이점이 있는지 궁금합니다!

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로 하게 했는데, 스타일링을 위한 부분은 Styled를 붙이는게 가장 좋은 것일까요?

Comment on lines 132 to 136
const MyLocationButton = styled.button`
// 왜 색 안바뀌지
color: ${(props: StHeaderProps) =>
props.isChoice ? theme.colors.skyblue : theme.colors.lightgreen};
`;
Copy link

Choose a reason for hiding this comment

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

왜 색 안 바뀌는지는 모르겠지만,,, 아래처럼도 SC props 타입 지정이 가능합니다!
제네릭으로 선언함으로써, 이후 css에서도 isChoice를 사용할 때마다 타입지정을 하지 않아도 됩니다

Suggested change
const MyLocationButton = styled.button`
// 왜 색 안바뀌지
color: ${(props: StHeaderProps) =>
props.isChoice ? theme.colors.skyblue : theme.colors.lightgreen};
`;
const MyLocationButton = styled.button<{ isChoice: boolean }>`
color: ${({ isChoice }) =>
isChoice ? theme.colors.skyblue : theme.colors.lightgreen};
`;

Copy link

@Happhee Happhee Jun 21, 2022

Choose a reason for hiding this comment

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

const colors = {
    black: "#333333",
    white: "#ffffff",
    lemonchiffon: "#fffacd",
    gray: "#6B7280",
    skeleton: "#808080",
    skyblue: "87ceeb",
    lightgreen: "99f299",
};

# 붙여주세요,,,,,

Comment on lines 39 to 44
background-color: lemonchiffon;

& h1 {
margin-top: 2rem;
}
`,
Copy link

Choose a reason for hiding this comment

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

해당 하는 & h1 을 쓰기보다는 h1 태그마다 css 선언을 해주는 것이
이후에 문제가 생겼을 때 원인 규명(유지보수)에 더 좋다고 생각합니다

중복 제거 등으로 정말로 필요할 때, & > h1 자식 선택자 정도로 사용하는 것이 필요해 보여요

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 +12 to +13
// 이건 대체 뭔뜻일까
if (isSearch) return new Array(10).fill(1).map((_, i) => <Skeleton key={i} />);
Copy link

Choose a reason for hiding this comment

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

Main 페이지에서 선언ㅇ한 isSearch state로
Header 컴포넌트에서 (handleIsSearch) 검색을 시작할 때 true로, 끝날 때 false로 변경시켜줍니다
그래서 검색 중일 때 스켈레톤 ui 를 보여줌으로써 로딩 중을 표시해주는 코드가 아닐까 생각합니다

Copy link

@Happhee Happhee left a comment

Choose a reason for hiding this comment

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

멋진 혠니~.~

const searchInput = searchRef.current;
searchInput.disabled = !searchInput.disabled;
setIsLocation((prev) => !prev);
if (searchRef.current !== null) {
Copy link

Choose a reason for hiding this comment

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

Suggested change
if (searchRef.current !== null) {
if (searchRef.current)

로 해도 될것같은느낌~.~

Comment on lines 26 to 27
//useRef의 타입을 어떻게 정의해여!?!!?
const searchRef = useRef<HTMLInputElement>(null);
Copy link

Choose a reason for hiding this comment

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

잘 지정한거 아니에여?!!!

Comment on lines +2 to +7
id?: number;
place_url?: string;
place_name?: string;
road_address_name?: string;
distance?: string;
phone?: string;
Copy link

Choose a reason for hiding this comment

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

그러게!!!!!!!!머야머야 왜머야!!!!!!

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

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

Successfully merging this pull request may close these issues.

3 participants