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

사용자 프로필 바 컴포넌트 구현 #152

Merged
merged 24 commits into from
Jul 31, 2024
Merged

Conversation

jaeml06
Copy link
Contributor

@jaeml06 jaeml06 commented Jul 31, 2024

PR의 목적이 무엇인가요?

프로필 사진의 액자 컴포넌트(ProfileFrame)와 프로필 사진의 액자 컴포넌트와 사용자 이름을 합친 ProfileBox를 만들었습니다,
또한 ProfileBox의 리스트를 만드는 ProfileList를 구현했습니다.

SVG처리를 위해 storybook의 main.ts, webpack.common.js와 custom.d.ts를 수정하였습니다.
SVG를 사용할 때

import EmptyProfile from '@_common/assets/empty_profile.svg';

위의 경우는 컴포넌트로 import하는 경우고

import EmptyProfile from '@_common/assets/empty_profile.svg?url';

위의 경우는 문자열로 import하는 경우이다.

이슈 ID는 무엇인가요?

설명

image

질문 혹은 공유 사항 (Optional)

@jaeml06 jaeml06 added FE 프론트엔드 관련 이슈입니다. 🌱 기능추가 feature (새로운 기능 구현) labels Jul 31, 2024
Copy link
Contributor

@cys4585 cys4585 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ~ 사소한 부분을 찾아서 댓글로 남겼어요 :)

interface ProfileCardProps {
profile: Participation;
}
export default function ProfileBox(props: ProfileCardProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ProfileBox -> ProfileCard
컴포넌트 이름이 수정이 안돼있어요!!!!!!

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

우리 스타일링을 위한 변수 네이밍에 대한 컨벤션을 두면 좋을 것 같아요~
저는 -Style 이렇게 뒀었어요.
이렇게 하자는 건 아니고, 정해지기만 하면 좋겠습니다 :0

지금 생각해보니 사용처에서 S.- 이렇게 쓰니까 -Stylesuffix도 빼는 게 좋겠네요 ㅎㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 이따구로 했는데 괜찮나요

사용처
image

스타일
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 한번 이야기 해봐요

import { css } from '@emotion/react';

export const ProfileContanier = css`
scrollbar-width: none; /* Firefox 현재 no-unsupported-browser-features 경고 발생 중 추후 확인 필요 */
Copy link
Contributor

Choose a reason for hiding this comment

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

주석에 TODO 적어두면 나중에 별도로 모아서 확인할 수 있습니다!

@@ -6,6 +6,11 @@ declare module '*.woff2' {
const src: string;
export default src;
}

declare module '*.svg?url' {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

align-items: center;
justify-content: center;

width: ${width}rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 아예 문자에 rem을 박아버렸는데요
예를 들어 width:'7rem'과 같이요

이 부분을 조정해보면 좋을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

밑에 width을 계산하는 부분이 있어서 rem으로 받으면 rem을 분리하고 숫자로 만든 다음 계산을 해야해서 일단 구현의 편의상 이와 같은 방식으로 구현했습니다. 다음 구현때 수정하도록 하겠습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 이따구로 했는데 괜찮나요

사용처
image

스타일
image

export default function ProfileFrame(props: ProfileFrameProps) {
const { width, height, onError, role, ...args } = props;

const handleError = (
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 이게 싫어서 빈 프로필을 png로 두었어요.

이미지가 없는 에러가 나면 이미지 에러=>js 단에서 onError 수행=>이미지 변경된 컴포넌트 변경=>재렌더링=>재패칭 인데

background로 url을 두 개 두게되면

css에서 이미지 에러=>바로 재패칭 이렇게 되니깐요

물론 svg가 가벼워서 이 방식이 처음에는 더 이득일 수도 있겠네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이것도 한번 다시 생각해볼 필요가 있겠네요. 아마 지금은 svg를 url로 사용할 수 있으니 가능할 것 같아요

export const Default: Story = {
args: {
participants: [
{
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
Contributor Author

Choose a reason for hiding this comment

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

그런 방법도 있네요. refact 과정에 반영하도록 하겠습니다.

@jaeml06 jaeml06 merged commit b693374 into develop-frontend Jul 31, 2024
1 check passed
@jaeml06 jaeml06 deleted the feat/#144 branch July 31, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 관련 이슈입니다. 🌱 기능추가 feature (새로운 기능 구현)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants