Conversation
msbelaid
left a comment
msbelaid
left a comment
There was a problem hiding this comment.
Awesome! thanks for your contribution
Left some comments
app/src/main/java/app/plugbrain/android/ui/designsystem/components/card/PlugCard.kt
Outdated
Show resolved
Hide resolved
| // Challenge text | ||
| Text( | ||
| text = challengeText, | ||
| style = MaterialTheme.typography.displayLarge.copy(fontWeight = FontWeight.Bold), |
There was a problem hiding this comment.
Let's make the text auto resizable so that if it exceeds the available width, it shrinks instead of overflowing. This might require upgrading the Jetpack Compose library. If the autoSize property isn’t recognized, please keep the current implementation as it is, I’ll update it after upgrading the library..
| style = MaterialTheme.typography.displayLarge.copy(fontWeight = FontWeight.Bold), | |
| style = MaterialTheme.typography.displayLarge.copy(fontWeight = FontWeight.Bold), | |
| maxLines = 1, | |
| autoSize = TextAutoSize.StepBased( | |
| minFontSize = MaterialTheme.typography.headlineMedium.fontSize, | |
| maxFontSize = MaterialTheme.typography.displayLarge.fontSize, | |
| stepSize = 1.sp | |
| ) |
| value = inputValue, | ||
| onValueChange = onInputChange, | ||
| onSubmit = onCheckAnswer, | ||
| placeholder = stringResource(R.string.plug_card_input_placeholder), |
There was a problem hiding this comment.
Please make the placeholder value generic by adding a parameter to PlugCard.
|
|
||
| // Check answer button | ||
| PlugButtonSecondary( | ||
| text = stringResource(R.string.plug_card_check_answer), |
There was a problem hiding this comment.
The CTA title should also be a compose parameter.
app/src/main/res/values/strings.xml
Outdated
| <string name="plug_card_input_placeholder">Write your answer</string> | ||
| <string name="plug_card_check_answer">Check your answer</string> |
There was a problem hiding this comment.
No need to add string resources here, as we want this to be customizable.
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| class PlugCardScreenshotTest { |
There was a problem hiding this comment.
Awesome, thanks for adding the screenshot tests.
Could you please record the screenshots by running ./gradlew recordPaparazziDebug? This will generate PNG images in app/src/test/snapshots/images/.
You can then commit these new images.
|
@jd-58 , just checking in to see if you’ve had a chance to make the updates. Let me know if you need any clarification or help. |
Thank you for the comments! My apologies for the delay. I will work on implementing these changes this week. |
…ents/card/PlugCard.kt
jd-58
force-pushed
the
jd-58/#104_challenge_card_component
branch
from
0481d62 to
ac4d4f4
Compare
|
I just pushed an update addressing your feedback. I had some issues with having my library out of sync with some changes in main I believe, so I attempted to rebase my changes to resolve the conflicts that arose. My apologies for the mix up! |
2 participants
This is in reference to issue #104. This PR implements the challenge card component, using the recently added PlugButton and PlugNumericalInput components, along with screenshot tests. Please let me know if you have any requested changes, and I will fix/implement your suggestions. Thank you!