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

메인화면 단순화 구현 #2

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Conversation

YugyeongChoi
Copy link
Collaborator

  • 구현한 것
  1. NavigationBarTitle
  2. NavigationBaritems: 하트 아이콘, 설정 아이콘
  3. 일기 작성 아이콘
  4. 다이어리 리스트: 사진, 제목, 태그, 날짜
  • 궁금한 점?
  1. NavigationBaritems를 Title이랑 같은 위치 선상에 놓는 것
  2. MVVM 형식이 이게 맞는지..
  3. 태그 같은 문자 배열을 Model에서 받아서 출력하는게 맞는지..
  4. 폰트 적용이 안 됨.. 폰트에서 Target MemberShip 체크 -> 빌드 failed -> (구글링해서 찾은 방법) 문제 있는 폴더 삭제 -> Target MemberShip 체크표시 사라짐의 반복..... ㅠㅠㅠㅠ

-앞으로 좀 더 할 것?

  1. MVVM이 아직 잘 구현이 안된거같아서.. 코드 고쳐보기
  2. 폰트 적용하기
  3. NavigationBaritem 고치기
  4. 삭제 아이콘 만들기

-p.s..
피그마에서 할당해준 패딩값으로 하니까 프리뷰에서는 자꾸 이상하게 나와서 일단 임의로 패딩값 잡았는데 그래도 될까? 양 옆 마진 정도는 같게 만들었오

Copy link
Member

@mooyoung2309 mooyoung2309 left a comment

Choose a reason for hiding this comment

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

PR올려주실때 View에 관련된 변동사항이 있으면, 영상이나 화면을 녹화 or 캡쳐하여서 업로드 부탁드려요 !

그리고 VIew를 분리하여 작성하는게 좋은데, 역시 밑에 참고한 레포에서 VIew 어떻게 작성하는지 참고하면 도움 많이 될겁니당.

참고 - depromeet/Pumping-iOS#131

Comment on lines +10 to +15
@font-face {
font-family: 'Pretendard';
font-weight: 900;
font-display: swap;
src: local('Pretendard Black'), url('./woff2/Pretendard-Black.woff2') format('woff2'), url('./woff/Pretendard-Black.woff') format('woff');
}
Copy link
Member

Choose a reason for hiding this comment

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

p5;

오 이거 처음보는데 설명가능할까요 ? 참고한 사이트가 있을까요 ?

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 +16 to +17
}

Copy link
Member

Choose a reason for hiding this comment

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

p3;

빈줄 있습니다.

Comment on lines +11 to +16
struct DiaryList {
var title: String
var tag: [String]
//var diaryimage : [Image]
var day : Int
}
Copy link
Member

Choose a reason for hiding this comment

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

p3;

모델은 별도의 폴더로 관리해주셔도 좋습니다. Models 파일안에 DiaryList를 넣는 것도 좋아보여요.
네이밍 피드백인데, 우선 다이어리가 1개 인데 리스트라는 이름은 피했으면 합니다.
Diary 정도가 좋아보이네요.

Comment on lines +18 to +21
//Views
//에러 해결...
struct DiaryListView: View {

Copy link
Member

Choose a reason for hiding this comment

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

p3;

일단 정렬 해주세요. xcode 단축키가 있는데, 아마 cmd + a 로 전체 선택후, cmd + cnt + i 였나 그럴거에요

Comment on lines +28 to +30
ZStack{
Image("background").ignoresSafeArea()
}
Copy link
Member

Choose a reason for hiding this comment

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

p3;

뒷 배경 때문이라면 다른 방식이 있어보입니다. 전체의 VStack의 Background 로 이미지를 채워넣는 방식을 고려해보세요.

.weight(.medium)
)

.kerning(0.288)
Copy link
Member

Choose a reason for hiding this comment

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

p3;

kerning은 어떤 프로퍼티인지 간략하게 설명가능할까요.

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 +104 to +109
ZStack(){
Button(action: {}) {
Image("pencil")
.frame(width: 42, height: 42)
.padding(EdgeInsets(top: 700, leading: 170, bottom: 120, trailing: 170))
}
Copy link
Member

Choose a reason for hiding this comment

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

p5;

저는 이거 ZStack으로 사용하는 것은 인정합니다. 이런 경우에 ZStack을 사용하는 것이 맞다고 생각해요.

@YugyeongChoi YugyeongChoi merged commit d08390c into main Jul 14, 2023
@YugyeongChoi YugyeongChoi deleted the feat/#1-Simplify-MainView branch July 14, 2023 15:42
@YugyeongChoi YugyeongChoi self-assigned this Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants