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

Step4 - 페이먼츠(카드 수정) #53

Open
wants to merge 18 commits into
base: oyj7677
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,12 @@
## 기능 목록
- [O] 카드사 목록 화면 구현
- [O] 선택한 카드사에 따라 카드 미리보기가 바뀌어야 한다.
- [] 카드사를 선택할 때 적절한 카드사 아이콘을 노출한다.
- [O] 카드사를 선택할 때 적절한 카드사 아이콘을 노출한다.
- [O] 카드 등록 시 카드에 카드사 이름이 표시된다.

# Step 4 - 페이먼츠(카드 수정)

## 기능 목록
- [O] 등록된 카드 선택시 카드 수정 화면으로 이동한다.
- [O] 카드 수정 시 변경이 일어나지 않으면 수정이 불가능하다.
- [O] 카드가 수정되면 카드 목록 화면에 변경사항이 반영된다.
2 changes: 2 additions & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
alias(libs.plugins.android.application)
alias(libs.plugins.jetbrains.kotlin.android)
id("kotlin-parcelize")
}

android {
Expand Down Expand Up @@ -68,4 +69,5 @@ dependencies {
androidTestImplementation(libs.androidx.ui.test.junit4)
debugImplementation(libs.androidx.ui.tooling)
debugImplementation(libs.androidx.ui.test.manifest)
implementation(kotlin("script-runtime"))
Copy link
Member

Choose a reason for hiding this comment

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

혹시 어떤 이유로 추가된걸까요~?

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.test.core.app.ApplicationProvider
import nextstep.payments.R
import nextstep.payments.data.BankType
import nextstep.payments.data.Card
import nextstep.payments.data.PaymentCardsRepository
import nextstep.payments.data.RegisteredCreditCards
Expand Down Expand Up @@ -54,14 +55,14 @@ class RegisteredCreditCardsScreenTest {
ownerName = "홍길동",
expiredDate = "12/24",
password = "123",
brandColor = Color(context.getColor(R.color.bc_card))
bankType = BankType.BC
),
Card(
cardNumber = "1234-5678-1234-5628",
ownerName = "홍길동",
expiredDate = "12/24",
password = "123",
brandColor = Color(context.getColor(R.color.bc_card))
bankType = BankType.BC
)
)
)
Expand Down Expand Up @@ -123,7 +124,7 @@ class RegisteredCreditCardsScreenTest {
ownerName = "홍길동",
expiredDate = "12/24",
password = "123",
brandColor = Color(context.getColor(R.color.bc_card))
bankType = BankType.BC
)
)
val registeredCreditCards = RegisteredCreditCards(
Expand All @@ -133,7 +134,7 @@ class RegisteredCreditCardsScreenTest {
ownerName = "홍길동",
expiredDate = "12/24",
password = "123",
brandColor = Color(context.getColor(R.color.bc_card))
bankType = BankType.BC
)
)
)
Expand Down
6 changes: 6 additions & 0 deletions app/src/main/java/nextstep/payments/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ class MainActivity : ComponentActivity() {
onAddCard = {
val intent = Intent(this, NewCardActivity::class.java)
launcher.launch(intent)
},
onCardClick = { card ->
val intent = Intent(this, NewCardActivity::class.java).apply {
putExtra("card", card)
}
launcher.launch(intent)
}
)
}
Expand Down
51 changes: 0 additions & 51 deletions app/src/main/java/nextstep/payments/NewCardViewModel.kt

This file was deleted.

3 changes: 2 additions & 1 deletion app/src/main/java/nextstep/payments/data/BankType.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package nextstep.payments.data

import androidx.annotation.ColorRes
import androidx.annotation.DrawableRes
import androidx.annotation.StringRes
import nextstep.payments.R

enum class BankType(
@StringRes val companyName: Int,
val logo: Int,
@DrawableRes val logo: Int,
@ColorRes val brandColor: Int,
) {
NOT_SELECTED(
Expand Down
9 changes: 6 additions & 3 deletions app/src/main/java/nextstep/payments/data/Card.kt
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package nextstep.payments.data

import androidx.compose.ui.graphics.Color
import android.os.Parcelable
import kotlinx.parcelize.Parcelize

@Parcelize
data class Card(
val id: Int,
val cardNumber: String,
val expiredDate: String,
val ownerName: String,
val password: String,
val brandColor: Color
)
val bankType: BankType
) : Parcelable
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,13 @@ object PaymentCardsRepository {
fun removeAllCard() {
_cards.clear()
}

fun editCard(oldCard: Card?, newCard: Card) {
val index = _cards.indexOfFirst { it.id == oldCard!!.id }
_cards[index] = newCard
}

fun createId(): Int {
return _cards.maxOfOrNull { it.id }?.plus(1) ?: 1
}
Copy link
Member

Choose a reason for hiding this comment

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

도메인에 너무 많은 신경을 쓰지 않고 학습 목표에 집중한 구현을 해주셨네요!


(선택사항) 그래도 혹시 구조를 개선하고 싶으시다면 다음 변경사항을 만들어 유지보수성을 높여보시는건 어떨까요?

  • oldCard의 id가 이미 newCard에 포함되어 있어서 oldCard를 받지 않아도 될 것 같아요.
  • 외부에 createId 함수가 공개될 필요가 있을까요?
  • 이외에도 프로젝트에서 !!를 불필요하게 활용하고 있다면 null safety하게 구현해볼 수 있을 것 같아요.

마지막 단계이니까 편하게 도전해보시죠!

}
50 changes: 28 additions & 22 deletions app/src/main/java/nextstep/payments/ui/PaymentCard.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package nextstep.payments.ui

import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
Expand All @@ -26,12 +28,14 @@ import nextstep.payments.ui.card.list.component.card.CardOwnerName

@Composable
fun PaymentCard(
brandColor: Color,
modifier: Modifier = Modifier,
brandColor: Color = Color(0xFF333333)
content: @Composable () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

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

의도대로 리뷰를 넘 잘 반영해주셨습니다 👍
이전에 비해 훨씬 컴포넌트 하나하나의 로직을 캡슐화되도록 설계되었어요.

) {
Box(
contentAlignment = Alignment.CenterStart,
modifier = modifier
.padding(top = 10.dp)
.shadow(8.dp)
.size(width = 208.dp, height = 124.dp)
.background(
Expand All @@ -49,19 +53,25 @@ fun PaymentCard(
shape = RoundedCornerShape(4.dp),
)
)
content()
}
}

@Composable
fun PaymentCard(
fun PaymentCardContents(
card: Card,
modifier: Modifier = Modifier,
content: @Composable () -> Unit = {}
onClick: (Card) -> Unit = {}
) {
Box(
modifier = Modifier.padding(top = 10.dp)
modifier = Modifier
.clickable { onClick(card) }
Copy link
Member

Choose a reason for hiding this comment

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

혹시 modifier 파라미터는 왜 사라졌을까요?

Copy link
Author

Choose a reason for hiding this comment

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

사용되지 않아서 지웠었는데 Modifier규칙과 관련된 글을 보니 항상 받아야한다고 하네요.
이전에 파라미터로 전달된 modifier를 내부 속성에 포함시켜 의도치 않는 상황을 만들었습니다.
modifier를 항상 넘겨야한다는 건 컴포넌트의 가장 외부 속성에는 항상 적용시켜줘야한다는 건가요?
이전에 설명해주신 modifier - Don't 사례입니다. 제가 말한 외부 속성이란 Box()를 뜻합니다.
image

) {
content()
Text(
text = card.bankType.name,
color = Color.White,
modifier = Modifier
.padding(start = 14.dp, top = 10.dp)
)

CardNumber(
cardNumber = card.cardNumber,
Expand Down Expand Up @@ -90,24 +100,20 @@ fun PaymentCard(

@Preview
@Composable
private fun PaymentCardPreview() {
PaymentCard(
brandColor = colorResource(id = BankType.BC.brandColor)
private fun NewPaymentCardPreview() {
val card = Card(
id = 1,
cardNumber = "1234-5678-1234-5678",
ownerName = "홍길동",
expiredDate = "12/34",
password = "123",
bankType = BankType.BC
)
}

@Preview
@Composable
private fun NewPaymentCardPreview() {
PaymentCard(
card = Card(
cardNumber = "1234-5678-1234-5678",
ownerName = "홍길동",
expiredDate = "12/34",
password = "123",
brandColor = colorResource(id = BankType.BC.brandColor)
),
modifier = Modifier.size(width = 208.dp, height = 124.dp),
content = { PaymentCard() }
brandColor = colorResource(id = BankType.BC.brandColor),
content = {
PaymentCardContents(card = card)
}
)
}
Loading