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

[#119] feat: 프로필 수정 화면 구현 #121

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

Conversation

SujinKim1127
Copy link
Member

🎯 Main Topic of feature / bug

  • 프로필 수정 화면 구현했습니다
  • 기존 useSignupForm 참고해서 useProfileUpdateForm 제작했습니다
    • 버튼 눌렀을때 제출되는 기능은 api 연결하면서 완성할예정입니다
  • 닉네임 중복확인 화면 만들면서 LabeledInputWithButton 컴포넌트 사용을 위해 중복확인 api만 연결했습니다

🧩 Description

image

🛠️ Related Issue or References

💡 Test Done & How to Test

📌 Checklist

  • 추가된 기능 혹은 해결한 bug를 크롬 시크릿 모드에서도 테스트 해보았습니다.
  • squash and merge로 설정되었나요?

⏰ TODOS

  • [ ]

Copy link

vercel bot commented Sep 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dnd-11th-3-frontend-gmi-design-system ❌ Failed (Inspect) Oct 6, 2024 0:54am

Copy link

netlify bot commented Sep 21, 2024

Deploy Preview for gongmuin failed. Why did it fail? →

Name Link
🔨 Latest commit 3a8c7b4
🔍 Latest deploy log https://app.netlify.com/sites/gongmuin/deploys/6702884ab849ab00086b90d1

@SujinKim1127 SujinKim1127 changed the base branch from main to feat/#117 September 21, 2024 11:21
@SujinKim1127 SujinKim1127 self-assigned this Sep 21, 2024
@SujinKim1127 SujinKim1127 linked an issue Sep 21, 2024 that may be closed by this pull request
Copy link
Member

@Zero-1016 Zero-1016 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 😊


return (
<div>
<div style={{ marginBottom: '36px' }}>
Copy link
Member

Choose a reason for hiding this comment

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

해당 속성만 인라인으로 주신 이유가 있으실까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

필요한 스타일 요소가 1개밖에 없어서 이렇게 작성을 했는데 style.css.ts에서 컴포넌트 선언하는게 더 좋을까요?
꼭 컴포넌트 선언을 해야할까? 라고 생각하면서 코드 작성했습니다
어떤게 더 좋을까요?
일관성 있게 컴포넌트 선언을 할까요?

Copy link
Member

Choose a reason for hiding this comment

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

다른 스타일의 경우 필요한 스타일이 하나만 있을때도 분리를 하셨기에 드린 질문이었습니다!

return (
<div className={pageContainer}>
<ProfileUpdateInputSection form={updateForm} />
<div className={marginBox}>
Copy link
Member

Choose a reason for hiding this comment

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

div 태그를 설정하지 않고 Button 태그내에 스타일을 전달해주는건 어떨까요? 불필요한 태그가 생기지 않아도 될 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

급한거 먼저 하느라 답변이 늦었네요 ㅠㅠ

혹시 불필요하다고 느끼신 이유가 있으신가요??
저는 컴포넌트를 항상 div로 감싸서 위치 코드를 div에 작성해서요!
그래야 뭔가 Button은 Button만의 스타일을 가지는 느낌이 들더라구요


export const useProfileUpdateForm =
(): UseFormReturn<ProfileUpdateFormValues> => {
const form = useForm({
Copy link
Member

Choose a reason for hiding this comment

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

아래처럼 바꾼다면 타입 단언을 사용하지 않아도 되지 않을까 싶습니다!

Suggested change
const form = useForm({
return useForm<ProfileUpdateFormValues>({

Base automatically changed from feat/#117 to main September 28, 2024 14:27
Copy link

Deploying gongmuin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3a8c7b4
Status:🚫  Build failed.

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] 프로필 수정 화면 구현
2 participants