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

[Feature/PreviewParameterProvider] Preview Parameter Provider 추가 #325

Merged

Conversation

chattymin
Copy link
Contributor

@chattymin chattymin commented Jun 5, 2024

Issue

Overview (Required)

  • 단순한 String과 같은 값들은 Preview Parameter Provider를 추가하지 않았습니다.
  • enum class, sealed class, boolean값처럼 확실한 변화를 주는 것으로 기준을 두어 Preview Parameter Provider를 추가했습니다.
  • 또한 list에서 단일값이 들어가는 경우와, 여러 리스트 값이 들어가는 경우 또한 구분이 필요하다 생각되어 Preview Parameter Provider를 추가했습니다.
  • Preview가 없어 생성
    • SessionChip
  • Preview Parameter Provider 추가
    • BookmarkCard
    • BookmarkItem
    • ContributorScreen
    • SessionChip
    • SessionCard
    • SessionDetailBookmarkStatePopup

Screenshot

BookmarkCard BookmarkItem ContributorScreen
SessionChip SessionCard SessionDetailBookmarkStatePopup

Copy link

github-actions bot commented Jun 5, 2024

Test Results

19 tests   19 ✅  5s ⏱️
11 suites   0 💤
11 files     0 ❌

Results for commit 7cdd4f3.

♻️ This comment has been updated with latest results.

@@ -77,14 +79,25 @@ internal fun BookmarkCard(
}
}

class RoomPreviewParameterProvider : PreviewParameterProvider<Room> {
Copy link
Member

Choose a reason for hiding this comment

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

모듈 내부에 만 있다보니 internal 키워두 있는것이 좋을것 같네요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 모듈 내부에서 사용하는 모든 PreviewParameterProviderinternal 키워드 추가했습니다 :)

BookMarkItemForPreview(isEditMode = true)
BookMarkItemForPreview(isEditMode = true)
BookMarkItemForPreview(isEditMode = isEditMode)
BookMarkItemForPreview(isEditMode = isEditMode)
Copy link
Member

Choose a reason for hiding this comment

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

이 부분까지 PreviewParameterProvider 까지는 불필요해보이네요.
그리고 하나면 이미 2개가 뜰것 같군요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 알겠습니다!
이전 코드로 롤백시키기 보다는 하나의 Preview내부에서 true하나, false 하나 하여 모든 case를 볼 수 있도록 수정하였습니다. 리뷰 감사합니다 :)

Copy link
Contributor Author

@chattymin chattymin Jun 5, 2024

Choose a reason for hiding this comment

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

추가로 BookmarkedPreviewParameterProvider 또한 단순한 true/false이기 때문에 같은 방식으로 수정하였습니다

Copy link
Member

@taehwandev taehwandev left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@taehwandev taehwandev merged commit 6688976 into droidknights:main Jun 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[개선] 일부 Preview를 PreviewParameterProvider를 활용해 관리하기
2 participants