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

#21: 편집 화면 전까지 회원가입 화면 추가 #25

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

flash159483
Copy link
Contributor

1. 📄 관련된 이슈 및 소개

#21

2. 🔥 변경된 점

아래 화면 추가

  • 학교 선택
  • 학년 선택
  • 반 선택
  • 성별 선택
  • 이름 선택

3. ✅ 필수 체크 사항

길어지기 전에 한번 나눠요

4. 📸 작업물 사진 공유(선택)

5. 💡알게된 혹은 궁금한 사항

@flash159483 flash159483 added 🌱기능🌱 새로운 기능을 추가해요 ! 🍩브라우니🍩 24기 정승원 🌟머지 해주세요🌟 코드 리뷰가 완료된 뒤 PR을 올린사람이 Merge를 하면 되는 단계입니다. labels Jul 9, 2024
@flash159483 flash159483 requested a review from jeongjaino July 9, 2024 12:05
@flash159483 flash159483 self-assigned this Jul 9, 2024
Copy link
Member

@jeongjaino jeongjaino left a comment

Choose a reason for hiding this comment

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

오빠 미안해. 많이 늦었어. 히힣

@@ -16,7 +16,8 @@
android:name=".MainActivity"
android:exported="true"
android:label="@string/app_name"
android:theme="@style/Theme.WeSpot">
android:theme="@style/Theme.WeSpot"
android:windowSoftInputMode="adjustResize">
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
Contributor Author

Choose a reason for hiding this comment

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

keyboard가 올라오면 화면을 사이즈를 맞춰서 button이 키보드 위에 올라오게 하는 옵션이에요

Comment on lines +44 to +46
.wrapContentHeight()
.padding(vertical = 8.dp)
.clip(WeSpotThemeManager.shapes.medium)
Copy link
Member

Choose a reason for hiding this comment

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

그냥 궁금한 내용인데요.!

디자인 시스템 내의 아이템 내 버티컬 패딩이 아닌,

LazyColuimn 내에 contentPadding을 사용할 수도 있지 않을까요 ?

승원장의 생각도 궁금하네용~ 나중에 패딩 값 다른 리스트 나오면 대처하기 쉽지 않을까요 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제 생각에는 다른 패팅 값이 있는 리스트가 나올 수가 없다고 생각하여 여기에 적용하긴 했어요.
조금더 다양한 상황을 대처하려면 column에 추가하는게 맞다고 생각해요.

Comment on lines +16 to +30
fun WSBottomSheet(
closeSheet: () -> Unit,
sheetState: SheetState = rememberStandardBottomSheetState(),
content: @Composable () -> Unit
) {
ModalBottomSheet(
onDismissRequest = closeSheet,
sheetState = sheetState,
shape = RoundedCornerShape(topStart = 25.dp, topEnd = 25.dp),
containerColor = WeSpotThemeManager.colors.bottomSheetColor,
dragHandle = null,
modifier = Modifier.navigationBarsPadding()
) {
content.invoke()
}
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

Choose a reason for hiding this comment

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

dragHandle은 어떤 속성인가요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bottomsheet 위에 있는 조금만한 막대기에요. 디자인에 없어서 null로 설정했어요.

Comment on lines +41 to +43

val state by viewModel.collectAsState()
val action = viewModel::onAction
Copy link
Member

Choose a reason for hiding this comment

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

collectAsState보다 collectAsStateWithLifecycle()은 어떨까요 ???

따로 사용하신 이유가 있을까요!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

없던데요 orbit viewModel에는 그 api가 없어요

Comment on lines +102 to +104
modifier =
Modifier
.weight(1f)
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

Choose a reason for hiding this comment

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

다음 피알에 린트 수정 반영되어 있어요... ㅠㅠ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

빨리 해 줘

contentAlignment = Alignment.Center,
) {
Column(
modifier = Modifier.padding(start = 30.dp, end = 30.dp, top = 30.dp, bottom = 17.dp),
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

Choose a reason for hiding this comment

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

애매하이 저도 안드리긴했는데

Copy link
Contributor Author

Choose a reason for hiding this comment

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

padding 슬퍼용

Comment on lines +6 to +19
data class OnSchoolSearchChanged(val text: String) : AuthAction()

data class OnSchoolSelected(val school: SchoolItem) : AuthAction()

data class OnGradeChanged(val grade: Int) : AuthAction()

data class OnGradeBottomSheetChanged(val isOpen: Boolean) : AuthAction()

data class OnClassNumberChanged(val number: Int) : AuthAction()

data class OnGenderChanged(val gender: String) : AuthAction()

data class OnNameChanged(val name: String) : AuthAction()
}
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

Choose a reason for hiding this comment

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

어때요 ?? 승원님도 처음아니에요 MVI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이번이 세번째에요

Comment on lines +136 to +139
LaunchedEffect(focusRequester) {
focusRequester.requestFocus()
delay(10)
keyboard?.show()
Copy link
Member

Choose a reason for hiding this comment

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

오 10초..! 이거 따로 요구사항인가요? 아니면

따로 승원님이 보기 편해서 넣은걸까용??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

바로 뜨는건 이상하다 생각해서 딜레이 조금 넣어요

@flash159483 flash159483 merged commit 3bb6b87 into develop Jul 10, 2024
5 of 24 checks passed
@flash159483 flash159483 deleted the feature/flash159483/#21 branch July 10, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟머지 해주세요🌟 코드 리뷰가 완료된 뒤 PR을 올린사람이 Merge를 하면 되는 단계입니다. 🌱기능🌱 새로운 기능을 추가해요 ! 🍩브라우니🍩 24기 정승원
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants