Revamp the "Market" page #41
Conversation
| */ | ||
| public class EntitySubscription<S : EntityState> internal constructor( | ||
| entityType: Class<S>, | ||
| client: DesktopClient, | ||
| id: Message | ||
| id: Message, | ||
| stateAccessor: (S?) -> Unit = {} |
There was a problem hiding this comment.
I think it's better to rename stateAccessor to beforeObserver (as in subscribeToEntity) or beforeUpdate because it will be called every time before the state is updated with the previous state.
| * The input component that supports displaying a tip. | ||
| * | ||
| * @param value the input text to be shown in the text field | ||
| * @param onChange the callback that is triggered when the input's value change |
There was a problem hiding this comment.
| * @param onChange the callback that is triggered when the input's value change | |
| * @param onChange the callback that is triggered when the input's value changes |
| start = if (leadingIcon == null) 16.dp else 5.dp, | ||
| end = 16.dp, | ||
| top = if (leadingIcon == null) 8.dp else 2.dp, | ||
| bottom = if (leadingIcon == null) 8.dp else 2.dp, |
There was a problem hiding this comment.
I think it's weird if leadingIcon is a @Composable function with no preset size, but the padding in this component depends on it existing with a specific size.
| ) { | ||
| Column( | ||
| Column ( |
There was a problem hiding this comment.
| Column ( | |
| Column( |
IntelliJ IDEA formats code without such spaces, maybe you should format this file.
| * Represents the search field. | ||
| * | ||
| * @param value the text to be shown in the search field | ||
| * @param onChange the callback that is triggered when the search value change |
There was a problem hiding this comment.
| * @param onChange the callback that is triggered when the search value change | |
| * @param onChange the callback that is triggered when the search value changes |
| ShareItem( | ||
| model = model, | ||
| share = share, | ||
| previousPrice = previousShares?.get(index)?.price |
There was a problem hiding this comment.
I think, this access to the previousShares by index produced a bug on .gif from the PR description, when price differences are not the same in ShareList and on ShareProfile.
| * @param previousPrice the previous price of this share | ||
| */ | ||
| @Composable | ||
| private fun MainItemContent( |
There was a problem hiding this comment.
I think it's better to rename MainItemContent to ShareItemContent because it strongly depends on the Share and corresponds to the other naming.
| @@ -150,4 +155,5 @@ private object Colors { | |||
| val DarkGrey = Color(0xff5b595f) | |||
| val Red = Color(0xffff3b30) | |||
| val Scrim = Color.Black.copy(alpha = 0.5f) | |||
| val Green = Color(52, 199, 89) | |||
There was a problem hiding this comment.
Consider using the same color notation in all places, such as HEX(0xaarrggbb) as in the colors above.
There was a problem hiding this comment.
@Artem-Semenov-dev LGTM except for a single comment. Please have a look.
| * @throws IllegalArgumentException if there is no `symbol` defined for this `Currency` | ||
| */ | ||
| private fun Currency.symbol(): String { | ||
| val valueDescriptor = CurrencyDescriptor.findValueByName(this.name) |
There was a problem hiding this comment.
All of these things right up to
if (rawOptions.hasExtension(MoneyProto.currency)) {
val resolvedOptions = rawOptions!!.getExtension(MoneyProto.currency)
return resolvedOptions.symbol
}
are definitely constant within JVM (or any other environment which does not allow hot-reload at run-time).
Also here, we may safely assume that each currency defined has this (currency) option defined. So even this hasExtension(...) condition is constantly true.
Please have some more optimisation done.
There was a problem hiding this comment.
@Artem-Semenov-dev as discussed, let's improve the API of that descriptor-related routine even more.
…o accept the parameter of the `Currency` type.
Configure Kotlin for the project
This PR changes the design of the "Market" page.
General view of the "MarketPage":
View of the search field:
View of the purchase modal window:
View of the pop-up with purchase result: