Skip to content

[Design] Add SettingView#33

Open
LiSeul wants to merge 2 commits into
developfrom
design/settingLayout
Open

[Design] Add SettingView#33
LiSeul wants to merge 2 commits into
developfrom
design/settingLayout

Conversation

@LiSeul
Copy link
Copy Markdown
Collaborator

@LiSeul LiSeul commented May 25, 2023

Motivation 🥳 (코드를 추가/변경하게 된 이유)

SettingView 추가

Key Changes 🔥 (주요 구현/변경 사항)

  1. SettingView 추가
  2. ContentView SettingView아이콘 및 이동경로 수정

ToDo 📆 (남은 작업)

없음

ScreenShot 📷 (참고 사진)

image

To Reviewers 🙏 (리뷰어에게 전달하고 싶은 말)

드디어 추가했습니다 :)

Reference 🔗

ContentView SettingView아이콘 수정하였는데 gearshape.fill 사용 안했는데 fill 아이콘처럼 떠서 고민입니다.ㅠ

Close Issues 🔒 (닫을 Issue)

Close #24

Copy link
Copy Markdown
Collaborator

@ikanic ikanic left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.
딱히 문제가 될만한 부분은 없고, 작은 제안 사항 하나와 탭뷰 부분에 아이콘에 관한 코멘트 하나 남겨 두었습니다.
참고하시면 좋을 것 같아요.

Comment thread Nav/ContentView.swift
SettingView()
.tabItem {
Image(systemName: "plus")
Image(systemName: "gearshape")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

해당 부분 탭 아이콘에 fill을 사용하고 싶지 않으시다면, environment value 중에서 symbolVariant에 관해서 찾아보시면 좋을 것 같습니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

해당부분 좀더 찾아보고 진행하겠습니다 ㅠㅠ

Copy link
Copy Markdown
Collaborator Author

@LiSeul LiSeul Jun 8, 2023

Choose a reason for hiding this comment

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

Teb아이콘이 모두 fill 이여서... 흠..... 회의떄 말씀드리겠슘다! 😀

Comment thread Nav/View/SettingView.swift Outdated
Comment on lines +39 to +53
.alert(isPresented: $isVersionAlert) {
Alert(title: Text("버전 정보"), message: Text("현재 버전: 1.0"), dismissButton: .default(Text("확인")))
}

Button(action: {
self.isResetAlert = true
}) {
Text("초기화")
}
.font(.subheadline)
.foregroundColor(.black)
.alert(isPresented: $isResetAlert) {
Alert(title: Text("초기화"), message: Text("초기화 하시겠습니까?"), primaryButton: .destructive(Text("초기화"), action: {
// 초기화 로직 구현
}), secondaryButton: .cancel(Text("취소")))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alert들이 반복된다면 Alert를 띄우는 변수를 하나로 통일하고, Alert들을 하나의 컴포넌트로 뺀 후에, 내부 내용과 동작만 다르게 할 수 있도록 하면 어떨까요?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

넵! 변경 진행하겠습니다!

Copy link
Copy Markdown
Collaborator Author

@LiSeul LiSeul Jun 8, 2023

Choose a reason for hiding this comment

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

d62de0e 공통 버튼을 담당하는 SettingButton 추가

@LiSeul LiSeul self-assigned this Jun 8, 2023
@LiSeul LiSeul changed the title Add SettingView [Design] Add SettingView Jun 8, 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.

[Design] Setting UI 구현

2 participants