Jordan/1754 forms #1807
Jordan/1754 forms #1807
Conversation
| @@ -126,7 +126,6 @@ export default { | |||
|
|
|||
| .tm-form-msg.tm-form-msg--error { | |||
| color: var(--danger); | |||
| position: absolute; | |||
There was a problem hiding this comment.
@jbibla important as the styling was bad for the message on the signup page
There was a problem hiding this comment.
noooooooooooooooooooo
i will fix the sign up page
| titleMaxLength: 64, | ||
| descriptionMinLength: 1, | ||
| descriptionMaxLength: 200, |
There was a problem hiding this comment.
do we actually need a descriptionMaxLength?
There was a problem hiding this comment.
@fedekunze @cwgoes do we have a max length in the SDK?
There was a problem hiding this comment.
AFAIK yes, the body will be cut after some charachters, but I don't know the exact size of it
There was a problem hiding this comment.
it would be nice if this matched the size from the SDK. but we can adjust later.
There was a problem hiding this comment.
There's a general size limit for messages, but not a specific one for description.
| @@ -188,8 +205,7 @@ export default { | |||
| }, | |||
| amount: { | |||
| required, | |||
| isInteger, | |||
| between: between(1, this.balance) | |||
| between: x => x >= 1 && x <= this.balance | |||
There was a problem hiding this comment.
why not use the between validator from vuelidate?
There was a problem hiding this comment.
probably didn't work well
There was a problem hiding this comment.
I think it would not accept 1
There was a problem hiding this comment.
given that we are here I will propose to fix a thing.
If my balance is 0 this form let me submit 1 succesfully, send the TX, and fail miserably.
I would like fail earlier and this between should be (this.balance > 0 ? 1 : 0, ...)
Or in the new format either this.balance > 0 && ...same...
There was a problem hiding this comment.
I.e. in app/src/renderer/components/wallet/SendModal.vue it's done in this way
| </tm-form-group> | ||
|
|
||
| <tm-form-group | ||
| :error="$v.amount.$dirty && $v.amount.$invalid" |
There was a problem hiding this comment.
| :error="$v.amount.$dirty && $v.amount.$invalid" | |
| :error="$v.amount.$error && $v.amount.$invalid" |
| /> | ||
| <tm-form-msg | ||
| v-if="!$v.amount.between && amount > 0" | ||
| v-if="$v.amount.$dirty && $v.amount.$invalid && !$v.amount.between" |
There was a problem hiding this comment.
| v-if="$v.amount.$dirty && $v.amount.$invalid && !$v.amount.between" | |
| v-if="$v.amount.$error && $v.amount.$invalid && !$v.amount.between" |
…into jordan/1754-forms
…into jordan/1754-forms
| <tm-form-msg | ||
| v-if="!$v.amount.between && amount > 0 && balance > 0" | ||
| v-if=" | ||
| $v.amount.$error && !$v.amount.between && amount > 0 && balance > 0 |
There was a problem hiding this comment.
on the other modals we're using v.amount.$error && !$v.amount.between && amount === 0 we should stick with one for consistency
There was a problem hiding this comment.
uhh it seems we have different msg types for required and between on the other modal, can we make that consistent ?
| <span class="input-suffix">{{ denom }}</span> | ||
| <tm-field id="amount" v-model="amount" type="number" /> | ||
| <tm-form-msg | ||
| v-if="$v.amount.$error && !$v.amount.between && amount === 0" |
There was a problem hiding this comment.
I took the above example from here ^^
| min-width: 9rem; | ||
| } | ||
| </style> | ||
| <style></style> |
There was a problem hiding this comment.
| <style></style> |
| <tm-btn | ||
| v-else | ||
| id="send-btn" | ||
| :disabled="$v.fields.$invalid" |
There was a problem hiding this comment.
this shouldn't be disabled as with the other modals
| @@ -76,7 +73,7 @@ export default [ | |||
| path: `/wallet/send/:denom?`, | |||
There was a problem hiding this comment.
curious why we need the ?
There was a problem hiding this comment.
means it is an optional param
|
@fedekunze didn't see you comments, will implement them on the follow up PR |
Closes #1754
Closes #1803
Description:
❤️ Thank you!
CHANGELOG.mdwith issue # and GitHub usernameFiles changedin the github PR explorer