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

[feat] PingleTag 추가 #198

Merged
merged 2 commits into from
Feb 26, 2024
Merged

[feat] PingleTag 추가 #198

merged 2 commits into from
Feb 26, 2024

Conversation

HAJIEUN02
Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 commented Feb 23, 2024

Related issue 🛠

Work Description ✏️

  • themes.xml에 PingleTag 추가

Screenshot 📸

image

Uncompleted Tasks 😅

N/A

To Reviewers 📢

backgroundTint로 하면 적용이 안 되고chipBackgroundColor로 설정해야 색상이 먹네용.. 새로운 사실 겟또!

@jihyunniiii
Copy link
Collaborator

PR 이름 수정해주세염 ~

Copy link
Collaborator

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

고생핑 ~ 다음부턴 이런 것도 feat으로 해주면 좋을 것 같아욤 ~ (add -> 진짜 파일 추가만 !!)

Comment on lines 111 to 120
<style name="Theme.Pingle.Tag" parent="Widget.MaterialComponents.Chip.Choice">
<item name="android:stateListAnimator">@null</item>
<item name="chipMinTouchTargetSize">0dp</item>
<item name="chipEndPadding">0dp</item>
<item name="chipStartPadding">0dp</item>
<item name="android:checked">false</item>
<item name="chipMinHeight">0dp</item>
<item name="backgroundTint">@null</item>
<item name="rippleColor">@android:color/transparent</item>
</style>
Copy link
Collaborator

Choose a reason for hiding this comment

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

따로 만들지 않고 이미 만들어둔 Theme.Pingle.Chip 을 사용하는 게 좋을 것 같네요 !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넹 적용 완!

<item name="rippleColor">@android:color/transparent</item>
</style>

<style name="Theme.Pingle.Tag.L" parent="Theme.Pingle.Tag">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<style name="Theme.Pingle.Tag.L" parent="Theme.Pingle.Tag">
<style name="Theme.Pingle.Chip.Tag.L" parent="Theme.Pingle.Chip">

이런 식으로 해주면 되겠죠?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

적용 완

Copy link
Collaborator

Choose a reason for hiding this comment

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

네이밍 좋네욤 ~~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

우헤헤

Comment on lines 133 to 153
<style name="Theme.Pingle.Tag.M" parent="Theme.Pingle.Tag">
<item name="cornerRadius">50dp</item>
<item name="textStartPadding">10dp</item>
<item name="textEndPadding">10dp</item>
<item name="android:paddingVertical">4dp</item>
<item name="android:clickable">false</item>
<item name="chipBackgroundColor">@color/g_10</item>
<item name="android:textColor">@color/pingle_green</item>
<item name="android:textAppearance">@style/TextAppearance.Pingle.Cap.Semi.12</item>
</style>

<style name="Theme.Pingle.Tag.S" parent="Theme.Pingle.Tag">
<item name="cornerRadius">50dp</item>
<item name="textStartPadding">8dp</item>
<item name="textEndPadding">8dp</item>
<item name="android:paddingVertical">3dp</item>
<item name="android:clickable">false</item>
<item name="chipBackgroundColor">@color/g_10</item>
<item name="android:textColor">@color/pingle_green</item>
<item name="android:textAppearance">@style/TextAppearance.Pingle.Cap.Semi.10</item>
</style>
Copy link
Collaborator

Choose a reason for hiding this comment

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

겹치는 게 많으니 parent theme을 하나 만드는 방법도 괜찮아 보이네요 ~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Theme.Pingle.Chip.Tag.All으로 만들어서 corenerRadius랑 backgroundTint 적용해씀다!

<item name="android:clickable">true</item>
<item name="chipBackgroundColor">@color/selector_pingle_tag_l_background</item>
<item name="android:textColor">@color/selector_pingle_tag_l_text_color</item>
<item name="android:textAppearance">@style/TextAppearance.Pingle.Sub.Semi.16</item>
Copy link
Collaborator

Choose a reason for hiding this comment

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

체크 되고 안 되었을 때 textAppearance가 다른 것 같아요 확인 부탁합니당

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네엥

@HAJIEUN02 HAJIEUN02 changed the title [add] #197 PingleTag 추가 [feat] PingleTag 추가 Feb 23, 2024
@@ -95,6 +95,41 @@
</item>
</style>

<!-- tag -->
<style name="Theme.Pingle.Chip.Tag.All" parent="Theme.Pingle.Chip">
<item name="cornerRadius">50dp</item>
Copy link
Collaborator

Choose a reason for hiding this comment

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

요것도 중복!!

Copy link
Collaborator

@Dan2dani Dan2dani left a comment

Choose a reason for hiding this comment

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

굳 ~~

Copy link
Member

@DoReMinWoo DoReMinWoo left a comment

Choose a reason for hiding this comment

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

고생핑

@HAJIEUN02 HAJIEUN02 merged commit e926d09 into develop Feb 26, 2024
1 check passed
@DoReMinWoo DoReMinWoo deleted the add-pingle-tag branch February 26, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[feat] PingleTag 추가
4 participants