-
Notifications
You must be signed in to change notification settings - Fork 2
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
[feat] 신규 단체 개설하기 뷰 구현 #199
Conversation
기존단체입장하기에서 안내페이지로 넘어오는 경우에 대한 분기처리 미구현
# Conflicts: # app/src/main/res/values/attrs.xml
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개최 프로세스랑 비슷하다 싶었는데 keyword 선택하는 부분이 새로워서 많이 배워가용
복사하기랑 공유하기도 화이또,,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
graphic1 말고 impossible이라든지 ban, prohibit 등등 다른 단어로 직관적인 표현을 해주시는 건 어떨까염?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 좋네요!!
피그마에서 추출할 때 파일명 그대로 사용했지만 직관적으로 바꿔보겠습니다~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기두요!!
app/src/main/res/values/attrs.xml
Outdated
@@ -4,6 +4,17 @@ | |||
<attr name="pingleEditTextTitle" format="reference|string" /> | |||
<attr name="pingleEditTextHint" format="reference|string" /> | |||
<attr name="pingleEditTextMaxLength" format="integer" /> | |||
<attr name="pingleEditTextFocusable" format="boolean" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와옹 이 부분은 잘 모르는데 어떤 식으로 다루는 건지 좀 배워갑니다 신기하네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
근데 Focusable은 왜 추가하신 건가염???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
복사하기 기능에서 유저가 editText를 수정하지 못하도록 막아야하는데 그러려면 focusable속성을 통해 false처리해야합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 대박핑!! 저번에 다은핑이 이거 막는 게 안 되어서 include를 통해 처리한 걸로 아는데 다은핑도 한 번 보면 좋겠네염 ㅋㅋ @Dan2dani
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 확인해봤는데 속성 추가 안 하고 editText.isEnabled 속성을 false로 지정하면 되네용?? 해보고 가능하면 속성 지워주셔도 될 것 같슴다 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jihyunniiii 따잇! 그런 쉬운방법이..... 해보고 수정할게요 ㅋㅎㅋㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jihyunniiii 아 그러네요 editText를 커스텀 해도 그안에서 editText를 그대로 사용하는거라 isEnabled나 isFocusable같은 속성을 코드에서 사용할 수 있네요 ㅋㅎㅋㅎㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오옹!
|
||
</data> | ||
|
||
<com.google.android.material.chip.Chip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
호오 이거 재활용하셨군요 똑똑이
app:layout_constraintEnd_toStartOf="@id/gl_end" | ||
app:layout_constraintStart_toEndOf="@id/gl_start" | ||
app:layout_constraintTop_toBottomOf="@id/tv_new_group_keyword_title" | ||
app:singleSelection="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단일 선택 로직 구현해야 하는 줄 알았는데 칩그룹으로 하면 이렇게 singleSelection 속성이 있군요.. 대박핑
<include | ||
android:id="@+id/include_new_group_topbar" | ||
layout="@layout/view_all_topbar_arrow_with_title" | ||
text="@{@string/new_group_topbar}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 include한 뷰에 text는 이런 식으로 집어넣어야 하는군용,, 동적으로 String 할당해주는 코드인데 왜 인자가 없지 했네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다.
app/src/main/AndroidManifest.xml
Outdated
<activity | ||
android:name=".presentation.ui.newgroup.NewGroupActivity" | ||
android:exported="false" | ||
android:screenOrientation="portrait" | ||
tools:ignore="LockedOrientationActivity" /> | ||
<activity | ||
android:name=".presentation.ui.newgroup.NewGroupInfoActivity" | ||
android:exported="false" | ||
android:screenOrientation="portrait" | ||
tools:ignore="LockedOrientationActivity" /> | ||
<activity | ||
android:name=".presentation.ui.newgroup.NewGroupAnnouncementActivity" | ||
android:exported="false" | ||
android:screenOrientation="portrait" | ||
tools:ignore="LockedOrientationActivity" /> | ||
<activity | ||
android:name=".presentation.ui.newgroup.NewGroupShareActivity" | ||
android:exported="false" | ||
android:screenOrientation="portrait" | ||
tools:ignore="LockedOrientationActivity" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 안내 페이지가 뜨는 경우에 대한 처리가 되어있지 않은 것 같습니다.
[신규 단체 개설 안내 페이지가 뜨는 경우]
- 핑글에 오신걸 환영합니다! 페이지에서 새로운 단체 등록을 눌렀을 경우 단체 개설 플로우로 가기 전 안내 페이지 보여줌
- 신규 단체 개설 플로우에서 상단에 인포 버튼을 누를 경우 안내 페이지 노출 (상단 바에 인포 버튼도 없는 것 같아요)
확인 부탁드립니다.
그리고 추가 적으로 해당 페이지 아래에서 위로 올라오는 애니메이션 들어가야 하는 걸로 알고 있어요 적용 부탁드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
노션에 애니메이션 정리글 보고 사용했는데 첫 진입시 화면전환이 안되는 문제와 아이콘을 두번 클릭해야 화면전환이 되더라구요! 이유는 잘 모르겠슴둥.,.
그래서 따로 찾아서 res/anim/slide_up.xml 파일 생성해서 Intent 할 때 바로 사용할 수 있도록 적용했습니다!
app/src/main/res/values/strings.xml
Outdated
@@ -65,6 +65,42 @@ | |||
<string name="join_group_success_description_group_name">%s에서</string> | |||
<string name="join_group_success_description">핑글 여정을 함께해보세요!</string> | |||
|
|||
<!-- new group --> | |||
<string name="new_group_topbar">신규 단체 개설</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
신규 단체 개설 -> 신규 단체 개설하기
android:layout_height="wrap_content" | ||
android:orientation="vertical" | ||
app:layout_constraintGuide_begin="@dimen/spacing26" /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적으로 gl_end도 만들어주어서 거기에 텍스트 end를 맞춰주는 게 좋겠다는 생각입니다 (기기대응 고려)
<ImageView | ||
android:id="@+id/iv_new_group_info_x" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="26dp" | ||
android:layout_marginEnd="12dp" | ||
android:src="@drawable/ic_all_x_24" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
패딩 추가해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예리하시네요..
this, | ||
object : OnBackPressedCallback(true) { | ||
override fun handleOnBackPressed() { | ||
navigateToNewGroup() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 뒤로가기 하면 새로운 그룹 추가 페이지로 가기로 정해졌나욤?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기획측에 물어보긴했는데 '신규 개설 단체의 홈화면' 으로 이동하는게 맞다고 하시는데 홈화면이 어느 화면을 지칭하는지 확실하지않아서 재확인중입니다!
setChipKeyword() | ||
} | ||
|
||
private fun setChipKeyword() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
칩 spacing 처리 해주셨나요? 안 보여서
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chip간의 간격은 xml 에서 chipSpacingHorizontal
속성으로 처리했습니다!
when (isChecked) { | ||
true -> chip.setTextAppearance(R.style.TextAppearance_Pingle_Sub_Semi_16) | ||
false -> chip.setTextAppearance(R.style.TextAppearance_Pingle_Body_Med_16) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
초기에 textAppearance 가 먹히지 않아서 처음에 칩 선택할 때 크기가 바뀌는 것 같아요 view_new_group_chip_keyword 여기에 있는 칩에 초기 textAppearance 값 추가해 주세요
@@ -25,7 +25,9 @@ class OnBoardingActivity : | |||
} | |||
|
|||
binding.includeOnboardingGroupNew.root.setOnClickListener { | |||
startActivity(navigateToWebView(NEW_GROUP_LINK)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하단 NEW_GROUP_LINK도 지워주세요
1 -> View.VISIBLE | ||
2 -> View.INVISIBLE | ||
3 -> View.GONE | ||
else -> View.GONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입을 만들어 주는 것도 좋을 듯요????
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋은데요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앱 돌려서 확인해보니까 스택관리가 제대로 안되어 있는 거 같네여 ~~ 확인부탁드립니다
private fun addListeners() { | ||
binding.btnNewGroupNext.setOnClickListener { | ||
when (binding.vpNewGroup.currentItem) { | ||
fragmentList.size - SUB_LIST_SIZE -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마지막 페이지일 때 넘어가게 하는걸 생각한 거 같은데 네이밍이 SUB_LIST_SIZE인 이유가 있을까영 ?!?! 한 번에 이해하기 좀 더 어려운 거 같아서여
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
신규 단체 안내 페이지는 새로 액티비티가 뜨는 방식 아닌가요? 뷰페이저 내부에 있는 게 아니라??? finish() 하면 기존에 있던 뷰가 그대로 보여질 것 같은데 아닌가요?
-> 그러네요! NewGroupActivity에서 onCreate할 때 안내페이지로 전환시키면 되는걸 어렵게 생각했네요!!
코드리뷰 반영했습니다!!
this, | ||
object : OnBackPressedCallback(true) { | ||
override fun handleOnBackPressed() { | ||
navigateToNewGroup() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기획측에 물어보긴했는데 '신규 개설 단체의 홈화면' 으로 이동하는게 맞다고 하시는데 홈화면이 어느 화면을 지칭하는지 확실하지않아서 재확인중입니다!
setChipKeyword() | ||
} | ||
|
||
private fun setChipKeyword() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chip간의 간격은 xml 에서 chipSpacingHorizontal
속성으로 처리했습니다!
1 -> View.VISIBLE | ||
2 -> View.INVISIBLE | ||
3 -> View.GONE | ||
else -> View.GONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋은데요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
같이 사용하기위해 이름을 바꾸려고하는데 네이밍이 고민되네요
selector_all_btn_background
라는 파일이 이미 있어서 selector_all_btn_g08_g02
라고 지으려고 하는데 어떤가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 좋네요!!
피그마에서 추출할 때 파일명 그대로 사용했지만 직관적으로 바꿔보겠습니다~!
app/src/main/res/values/attrs.xml
Outdated
@@ -4,6 +4,17 @@ | |||
<attr name="pingleEditTextTitle" format="reference|string" /> | |||
<attr name="pingleEditTextHint" format="reference|string" /> | |||
<attr name="pingleEditTextMaxLength" format="integer" /> | |||
<attr name="pingleEditTextFocusable" format="boolean" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
복사하기 기능에서 유저가 editText를 수정하지 못하도록 막아야하는데 그러려면 focusable속성을 통해 false처리해야합니다!
app/src/main/res/values/themes.xml
Outdated
<style name="Theme.Pingle.Button.Check" parent="Theme.Pingle.Button"> | ||
<item name="android:paddingHorizontal">10dp</item> | ||
<item name="android:paddingVertical">3dp</item> | ||
<item name="android:textAppearance">@style/TextAppearance.Pingle.Cap.Bold.10</item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와 진짜 꼼꼼히 봐주시네요 감사합니다!
<ImageView | ||
android:id="@+id/iv_all_topbar_arrow_with_title_info" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginEnd="12dp" | ||
android:src="@drawable/ic_all_info_24" | ||
android:visibility="invisible" | ||
app:layout_constraintBottom_toBottomOf="@id/tv_all_topbar_arrow_with_title_title" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintTop_toTopOf="@id/tv_all_topbar_arrow_with_title_title" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 그렇게 볼 수 있겠네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 무슨말인지 잘 모르겠어요!!
activity_new_group_share 파일이라 id가 00_new_group_share_00 으로 되어있는데 이걸로 부족할까요!?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
노션에 애니메이션 정리글 보고 사용했는데 첫 진입시 화면전환이 안되는 문제와 아이콘을 두번 클릭해야 화면전환이 되더라구요! 이유는 잘 모르겠슴둥.,.
그래서 따로 찾아서 res/anim/slide_up.xml 파일 생성해서 Intent 할 때 바로 사용할 수 있도록 적용했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니당~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하지은 이제 나를 너무 잘 안다 무섭다 무서워
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㄷ
class NewGroupKeywordFragment : | ||
BindingFragment<FragmentNewGroupKeywordBinding>(R.layout.fragment_new_group_keyword) { | ||
private val itemList = | ||
listOf(Item(1, "연합동아리"), Item(2, "교내동아리"), Item(3, "학생회"), Item(4, "대학교")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기두 다 상수화할 수 있지 않을까용?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 나중에 서버통신으로 불러올 거라 ㄱㅊ을 듯요 !!
@@ -48,6 +50,20 @@ class PingleEditText @JvmOverloads constructor( | |||
if (maxLength > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 상수화 해주세염!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오옹 여기는 제가 작성한 코드가 아니긴한데 분리해놓겟씀둥
app/src/main/res/anim/slide_up.xml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 애니메이션 첨 봐요 신기하다..
android:id="@+id/tv_new_group_create_name" | ||
android:layout_width="0dp" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="4dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전반적으로 가이드라인에만 dimen을 쓰시는 것 같은데 맞나요?! 나중에 디자인이 수정되는 경우를 고려하면 그게 맞는 것 같기도 하네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이게 더 좋다고 생각하긴 합니다 ~~
android:layout_width="match_parent" | ||
android:layout_height="match_parent"> | ||
|
||
<androidx.constraintlayout.widget.Guideline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가이드라인의 신 ㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿티비
object : OnBackPressedCallback(true) { | ||
override fun handleOnBackPressed() { | ||
navigateToHome() | ||
finish() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigateHome에 finish가 있는ㄴ데 또 추가해야 하나용??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오옹 그러네요!?
startActivity( | ||
this, | ||
ActivityOptions.makeCustomAnimation(this@NewGroupActivity, R.anim.slide_up, 0) | ||
.toBundle() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
쏘굿도리 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VisibilityType으로 변경 플리즈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기두 집에 가서 더 보겠움
<enum name="visible" value="0" /> | ||
<enum name="invisible" value="1" /> | ||
<enum name="gone" value="2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 Value 값을 Type에 넣어두고 사용해도 좋을 것 같습니다 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떻게 해야할지 감이 안와요 ㅠㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흠냐 이걸 다시 한 번 봐봤는데요 어차피 PingleEditText로 만드는 과정에서 Int로 변환시켜주는 과정이 있어야 하군요,,, 그럼 타입을 쓰는게 굳이,,, 싶은 생각도 들고 민우핑 생각은 어떠신가요,,,
# Conflicts: # app/src/main/java/org/sopt/pingle/presentation/ui/joingroup/JoinGroupSearchActivity.kt # app/src/main/res/values/strings.xml
Related issue 🛠
Work Description ✏️
Screenshot 📸
Recording_2024-02-25-23-08-40.mp4
Uncompleted Tasks 😅
To Reviewers 📢
공유하기, 초대코드, 중복확인 스낵바, 버튼 활성화는 서버통신할 때 구현하겠습니다!