ShapeShift integration #652

Merged
neb-b merged 7 commits from shapeshift into master 2017-12-06 01:33:41 +01:00
5 changed files with 14 additions and 21 deletions
Showing only changes of commit d043233094 - Show all commits

View file

@ -72,8 +72,7 @@ class ActiveShapeShift extends React.PureComponent<Props> {
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
<p>
Send up to{" "}
<span className="credit-amount--bold">
{originCoinDepositMax}{" "}
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
<span className="credit-amount--colored">{shiftCoinType}</span>
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
{originCoinDepositMax} {shiftCoinType}
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
</span>{" "}
to the address below.
</p>
@ -109,9 +108,9 @@ class ActiveShapeShift extends React.PureComponent<Props> {
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
</p>
</div>
)}
<div className="shapeshift__actions">
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
<div className="card__actions card__actions--only-vertical">
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
<Link
button="primary"
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
button={shiftState === statuses.COMPLETE ? "primary" : "alt"}
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
onClick={clearShapeShift}
label={
shiftState === statuses.COMPLETE ||

neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes
neb-b commented 2017-11-28 23:38:32 +01:00 (Migrated from github.com)
Review

Good idea

Good idea
kauffj commented 2017-11-30 02:42:13 +01:00 (Migrated from github.com)
Review

Try button="text" here.

Try `button="text"` here.
kauffj commented 2017-11-30 02:42:49 +01:00 (Migrated from github.com)
Review

Add "."

Add "."
kauffj commented 2017-11-30 02:46:52 +01:00 (Migrated from github.com)
Review

Add ".". Also, let's link to FAQ here again.

Add ".". Also, let's link to FAQ here again.
kauffj commented 2017-11-30 02:49:16 +01:00 (Migrated from github.com)
Review

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.

I'm not in love with "New" here - maybe just "Reset"? Though I don't like this either. As a (potentially weird) user, I have a desire for things to be back in their default state when "Done". But I don't necessary want to do a new one. I just want it back to where it was. Not sure on best name for this.
kauffj commented 2017-11-30 02:49:26 +01:00 (Migrated from github.com)
Review

Also this should probably be primary, whatever the label is.

Also this should probably be primary, whatever the label is.
neb-b commented 2017-12-04 18:24:37 +01:00 (Migrated from github.com)
Review

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better).

We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"

I'm not sure how much I like that button. What if they click it before actually sending? We would need some new bit of UI to say "We will keep pinging shapeshit to see if you actually sent it" (obviously worded better). We say "this will update automatically" and have a link to the shapeshift status page, I don't see any reason to create another shift status "user says they sent it but shapeshift hasn't received it"
liamcardenas commented 2017-12-04 21:14:00 +01:00 (Migrated from github.com)
Review

outside of the constructor, I think you need to add

continuousFetch: ?number;

as per https://flow.org/en/docs/types/classes/

outside of the constructor, I think you need to add `continuousFetch: ?number;` as per https://flow.org/en/docs/types/classes/
liamcardenas commented 2017-12-04 21:14:19 +01:00 (Migrated from github.com)
Review

this should get rid of all the flow fix mes

this should get rid of all the flow fix mes

View file

@ -86,7 +86,7 @@ export default (props: Props) => {
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
onBlur={handleBlur}
value={values.returnAddress}
errorMessage={errors.returnAddress}
hasError={touched.returnAddress && errors.returnAddress}
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
hasError={touched.returnAddress && !!errors.returnAddress}
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
/>
<span className="help">
<span>
@ -95,7 +95,7 @@ export default (props: Props) => {
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
{__("to this address if the transaction doesn't go through.")}
</span>
</span>
<div className="shapeshift__actions">
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
<div className="card__actions card__actions--only-vertical">
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
<Submit
label={__("Begin Conversion")}
disabled={isSubmitting || !!Object.keys(errors).length}

neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that
neb-b commented 2017-11-30 02:54:09 +01:00 (Migrated from github.com)
Review

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic input style to be more inline with the wunderbar

Yeah definitely not a fan of the current input styling. I was thinking this was fine for now since the PR is getting bigger and bigger. But might as well do it now and avoid more tech debt. I will update the generic `input` style to be more inline with the wunderbar
neb-b commented 2017-11-30 06:18:07 +01:00 (Migrated from github.com)
Review

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this:

Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle.

For this element (select), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this select element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the FormRow component uses).

If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change.

Would love to hear everyone else's thoughts. @liamcardenas @IGassmann

I think this should stay as just classes with regular html elements. I can make my case more tomorrow, but my main idea is this: Shared components are good for elements that can very a lot, but only with small differences. Buttons have many different possible styles (different background colors, borders, labels, etc). A lot of different variation, but the component is generally the same, a box with some text in the middle. For this element (`select`), there can be many different ways to present it. I think it definitely makes sense to use a shared components if there might be inline errors or something we want to keep the same across the app, but there won't be any errors on this `select` element. I just want the styling with text on the left and right, which is very easy just by using regular html elements (and the same classes that the `FormRow` component uses). If we start building out the shared components so much that they meet every possible use case, they can quickly become huge, un-maintainable files that can be terrible pain to update/change. Would love to hear everyone else's thoughts. @liamcardenas @IGassmann
neb-b commented 2017-12-04 17:38:54 +01:00 (Migrated from github.com)
Review

Yes we can display the actual rate. I'll add that

Yes we can display the actual rate. I'll add that

View file

@ -39,11 +39,6 @@ body
font-weight: 700;
}
.credit-amount--colored
{
color: var(--color-primary);
}
#main-content
{
margin: auto;

View file

@ -51,7 +51,7 @@
.card__actions {
margin-top: var(--card-margin);
margin-bottom: var(--card-margin);
user-select: none;
user-select: none;
}
.card__actions--bottom {
@ -74,6 +74,13 @@
}
}
.card__actions--only-vertical {
margin-left: 0;
margin-right: 0;
padding-left: 0;
padding-right: 0;
}
.card__content--extra-vertical-space {
margin: $spacing-vertical 0;
}

View file

@ -34,15 +34,7 @@
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
margin-left: 40px;
}
.shapeshift__actions {
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
padding-top: $spacing-vertical;
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
}
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
.shapeshift__submit,
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
.shapeshift__actions-help {
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
padding-top: $spacing-vertical / 2;
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
}
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
.shapeshift__link {
padding-left: 10px;
}
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
}
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍

neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍
neb-b commented 2017-11-29 04:54:58 +01:00 (Migrated from github.com)
Review

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?

Don't need this class. The success class is just to add the green color on the text. There isn't any styles that only do that, maybe it should be a more generic className?
neb-b commented 2017-11-29 05:00:49 +01:00 (Migrated from github.com)
Review

Yeah...

Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered.

Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading

Yeah... Can't think of better way to do it. Nothing bad could really happen, at least nothing worse than not having this. If someone changes the form content to be a different height, it will just "jump" to that height after it's rendered. The same thing would happen if we don't have this style, it will "jump" and increase the height when the form is rendered. Happy to change it if we can think of a better way. I just hate having content jumping when stuff is loading
neb-b commented 2017-11-30 06:35:16 +01:00 (Migrated from github.com)
Review

I'll keep the min-height on shapeshift__maxdeposit to avoid content jumping when we fetch new max limits after the user changes the origin coin

I'll keep the min-height on `shapeshift__maxdeposit` to avoid content jumping when we fetch new max limits after the user changes the origin coin
neb-b commented 2017-11-30 06:40:25 +01:00 (Migrated from github.com)
Review

I don't think it's necessary to use the card-xx styles for this. It doesn't have anything to do with the card style. It's just a small bit of markup that needs some padding. I think ideally we could just use a s-vertical-padding helper class or something similar. We can discuss this more along with the display-xx helper classes.

I don't think it's necessary to use the `card-xx` styles for this. It doesn't have anything to do with the `card` style. It's just a small bit of markup that needs some padding. I think ideally we could just use a `s-vertical-padding` helper class or something similar. We can discuss this more along with the `display-xx` helper classes.
neb-b commented 2017-11-30 06:41:42 +01:00 (Migrated from github.com)
Review

👍

👍
kauffj commented 2017-11-30 17:47:48 +01:00 (Migrated from github.com)
Review

card__content is the class I'd propose using instead.

`card__content` is the class I'd propose using instead.
neb-b commented 2017-11-30 19:04:15 +01:00 (Migrated from github.com)
Review

👍

👍