-
Notifications
You must be signed in to change notification settings - Fork 46
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
EuphonyTxPanel Component & Documentation #262
Conversation
// TODO : EuTxManager() should be changed to use singleton pattern | ||
val viewModel: TxPanelViewModel = viewModel(factory = TxPanelViewModelFactory(EuTxManager())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If #255 is merged, I need to change this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied the singleton pattern patch! 9be03e5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zion830 Thank you for implementing EuphonyTxPanel!
Your test code and documentation is cool 👍
I commented one thing and check please :)
#### Options | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WoW! So nice options 👍 It will help to customize EuphonyTxPanel
!
// TODO : EuTxManager() should be changed to use singleton pattern | ||
val viewModel: TxPanelViewModel = viewModel(factory = TxPanelViewModelFactory(EuTxManager())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TextField( | ||
modifier = Modifier | ||
.weight(1f) | ||
.fillMaxHeight(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested your EuphonyTxPanel
, and even if row has panelHeight
, your TextField
and Button
has fillMaxHeight()
, so it fills whole screen. So, I think TextField
and Button
should have panelHeight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for letting me know! I fixed it
15c0d37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EuphonyTxPanel
component implementation, documentation, and unit-test are perfect! 👍
This is the good starting point of TxPanel's component!
Related issue : #231
EuTxManager
. It also provides several options to custom design.Screenshot