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] fix: media 유틸리티 함수 수정 #632

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Conversation

chysis
Copy link
Contributor

@chysis chysis commented Sep 18, 2024


🚀 어떤 기능을 구현했나요 ?

  • 기존의 media 함수는 styled-components에 특화되어 사용 가능했습니다.
  • emotion에서는 styled-component 방식과 css 방식 모두를 사용 가능하기 때문에 호환성의 문제가 있었습니다.
  • styled와 css의 argument 타입이 다르기 때문에 하나의 함수로 이 둘을 모두 처리하는 것은 불가능하고, 재사용 가능한 미디어 쿼리 조건문을 생성하는 함수로 수정했습니다.

🔥 어떻게 해결했나요 ?

large: "@media (min-width: 1025px)"
medium: "@media (max-width: 1024px)"
small: "@media (max-width: 768px)"
xSmall: "@media (max-width: 425px)"
xxSmall: "@media (max-width: 320px)"

위와 같은 문자열을 반환하도록 함수를 수정했습니다.
함수 수정 과정에서 min-widthmax-width를 모두 사용해 범위를 구체적으로 명시했지만, 일부 breakpoint를 사용하지 않을 경우 스타일 코드의 중복이 발생하기 때문에 min과 max 둘 중 하나만을 사용하도록 했습니다.

// styled에서 적용
${media.xxSmall} {
  background-color: ${({ theme }) => theme.colors.red};
}

// css에서 적용
${media.medium}{
  background-color: ${theme.colors.red};
}

위와 같이 media를 사용 가능합니다. styled, css에서 모두 기존처럼 theme 사용 가능합니다.
media를 사용할 때 대괄호 표기법과 점 표기법을 모두 사용 가능한데, 익숙한 방식인 점 표기법을 그대로 사용하면 될 것 같습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

@chysis chysis added this to the 5차 스프린트 milestone Sep 18, 2024
@chysis chysis self-assigned this Sep 18, 2024
@chysis chysis linked an issue Sep 18, 2024 that may be closed by this pull request
@@ -29,7 +29,7 @@ export const Contents = styled.div`

box-sizing: border-box;
width: 100%;
max-width: ${({ theme }) => theme.breakpoints.desktop};
max-width: ${({ theme }) => theme.breakpoint.medium}px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

템플릿 리터럴 내부에서는 number 타입으로 지정하게 되면 px 단위가 자동으로 안 붙는 것으로 알고 있어서 따로 기입했습니다.

Copy link
Contributor

@BadaHertz52 BadaHertz52 left a comment

Choose a reason for hiding this comment

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

기존에 올린 pr에서 변경된 media 적용해봤어요
모두 문제 없이 반응형 디자인 반영되네요

연휴 기간동안 media 수정하느라 고생했어요 !!

Copy link
Contributor

@ImxYJL ImxYJL 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
Contributor

@soosoo22 soosoo22 left a comment

Choose a reason for hiding this comment

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

오! 이제 media 유틸 안에서도 theme을 사용할 수 있군요! 수고했어요 에프이!

@soosoo22 soosoo22 merged commit f3ccf7d into develop Sep 19, 2024
5 checks passed
@donghoony donghoony deleted the fe/fix/631-media branch September 26, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FE] 반응형 유틸리티 함수를 수정한다.
4 participants