search

OpenBazaar / openbazaar-desktop

OpenBazaar 2.0 Desktop Client (talks to openbazaar-go server daemon)
MIT License
647 stars 186 forks source link

Eth-Precision Variant Surcharge No Longer Defaults to Zero #1821

Closed jjeffryes closed 4 years ago

jjeffryes commented 4 years ago

When creating a listing with variants, the surcharge no longer defaults to zero, like it used to. This causes an error when the user tries to save their listing if they haven't manually entered zero into the surcharge fields for each variant combo.

(I confirmed that it auto-fills zero by default in master)

image

rmisio commented 4 years ago

Well, it wasn't by design that it autofills in master. It was because protobuf defaults integers to 0 and now that the number is a string, it defaults to an empty string.

We don't really default most other things. We leave it on the user to explicitly enter it otherwise they may gloss over a field and not even think to add in a value when they otherwise would want to.

I could work back in a default of zero, but really it's a UX decision on if that's what we want.

jjeffryes commented 4 years ago

My strong preference is to keep the experience the same, even if it means changing things in the code.

rmisio commented 4 years ago

Even if the experience you want to match is inconsistent with how we treat the vast majority of other fields in the app and it wasn't even an intentional experience, just a product of a limitation of the server's serialization lib?

jjeffryes commented 4 years ago

I think having defaults where they make sense is a good idea, if we don't do that in other places then it'd be nice to fix those other places too.

We could make it a server issue to put those defaults in, if that's better.

rmisio commented 4 years ago

Ok, if you want me to put this particular default back in, despite disagreeing because it breaks consistency, I'll do it.

If you want me to put defaults across the board or have the server do it, I am not comfortable with that. I think that would need to involve the UX team. I believe the UX team put a lot of thought on what fields should be required and whether those fields should have defaults. I did my best to follow the specs on that and only couldn't on the surcharge due to a server limitation.

If we're going to change the UX, I think it's a decision that should have the approval of the UX team.

jjeffryes commented 4 years ago

Just this one, so it stays consistent with the previous UX.

rmisio commented 4 years ago

fixed via e9df714e24afcad1513ee3f4d0aefc6c804f318b

rmisio commented 4 years ago

This fix is already merged into ethereum-master.