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

분실물 찾기 페이지들 퍼블리싱 #35

Merged
merged 24 commits into from
Jul 19, 2023
Merged

Conversation

Ubinquitous
Copy link
Member

작업사항

분실물 찾기와 관련된 목록 보기, 글 보기, 글 쓰기 페이지를 퍼블리싱했습니다.

스크린샷

스크린샷 2023-07-18 오후 7 50 56 스크린샷 2023-07-18 오후 7 49 50 스크린샷 2023-07-18 오후 7 49 44

@Ubinquitous Ubinquitous added the enhancement New feature or request label Jul 18, 2023
@Ubinquitous Ubinquitous requested review from J1min and 5ewon July 18, 2023 10:58
@Ubinquitous Ubinquitous self-assigned this Jul 18, 2023
@Team-INSERT Team-INSERT deleted a comment from sonarcloud bot Jul 18, 2023
@Ubinquitous
Copy link
Member Author

테스트용 코드로 작성해놓고 소나클라우드 적용하는바람에
string으로 조건을 구별해놓은 코드를 신뢰성 없는 코드로 인식해버려서 failed를 띄우네요..!!!
감안하고 봐주시면 감사하겠습니다 아래 PR도 동일합니다!

Copy link
Member

@5ewon 5ewon left a comment

Choose a reason for hiding this comment

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

수고하셨습니다

<Container>
<StateBox checked={checked} handler={onCheckLostFoundType} />
<Input label="글 제목" placeholder="글 제목을 입력해주세요" />
{checked === "lost" && (
Copy link
Member

Choose a reason for hiding this comment

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

lostfound.constant 사용하시는게 어떨까요?
이부분만 매직 리터럴을 허용하기도 애매하고, 따로 정의해주신거 있으니 그거 쓰면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 것 같네요 선언은 해두고 깜빡하고 있었습니다 반영했습니다!!

size={size}
onErrorDelete={onErrorDelete}
onError={() => setImgSrc(fallbackSrc)}
width={999}
Copy link
Member

Choose a reason for hiding this comment

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

이미지 width, height는 왜 고정되어 있는건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

이미지이다 보니 width와 height를 지정해주면 이미지가 찌부될 것을 고려하여 삽입된 이미지 자체를 담고 싶어서,
width를 n%로, height를 auto로 지정하고 싶었는데 해당 속성으로는 width와 height가 둘다 required 속성인 데다가
SafeNumber 속성만 삽입할 수 있어 width와 height를 고정해두고 스타일드 컴포넌트를 통해 width와 height를 지정했습니다..!!
더 괜찮은 방법 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

<Image
        alt={title}
        src={thumbnail}
        width={0}
        height={0}
        sizes="100vw"
        className="w-full h-auto"
/>

tailwind 기준으로 요런식으로 작성하면 원하는 동작을 할 것 같습니다~

Copy link
Member Author

@Ubinquitous Ubinquitous Jul 19, 2023

Choose a reason for hiding this comment

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

sizes라는 속성을 모르고 있었네요 수정 완료했습니다 감사합니다!

height={46}
/>
<Column justifyContent="center">
<Author>우빈우빈</Author>
Copy link
Member

Choose a reason for hiding this comment

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

const { data } = usePost()

이런식으로 예제 데이터 불러와주는 모델 만들고 거기서 불러와서 퍼블리싱 진행하면 나중에 서버 연동할 때, 모델만 변경하면 되서 좋을듯 합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

다른 PR로 한꺼번에 msw 적용 시도해보겠습니다!!

Copy link
Member

@J1min J1min left a comment

Choose a reason for hiding this comment

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

저번 PR과 마찬가지로 퍼블리싱이라 별다른 특이사항은 없네요~
수고하셨습니다!

@Ubinquitous Ubinquitous merged commit 3932f89 into main Jul 19, 2023
1 check passed
@Ubinquitous Ubinquitous deleted the layouts/lostfound branch November 27, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants