avatarUpload Logo

[kimispencer]

• added a Logo variant
• this variant behaves exactly the same as the avatarUpload but requires a different default background

[arthurxavierx]

If only avatarUpload were a LumiComponent we could make this work without changing anything in the library :p

[arthurxavierx]

I understand that this is meant to avoid breaking changes, but I'm thinking that a better design would be to maybe expose the style and a background prop that allows the parent to control these instead of adding to the Variant type. What do you think @kimispencer @spicydonuts?

I might be totally wrong, please note that I haven't taken the time to check what this hypothetical breaking change would affect.

[arthurxavierx]

The Avatar case is superfluous, as it's subsumed by _ here and has the same result.

[arthurxavierx]

The Avatar case is superfluous, as it's subsumed by _ down below and has the same result.

[maddie927]

I think I do agree with @arthurxavierx that adding those props would be a little more flexible, but I also don't want to add props we won't need later (assuming the switch to LumiComponent at some point), and this approach using the variant seems fine in the meantime. Whichever you prefer, though we probably want to merge tomorrow if possible.

[kimispencer]

I also agree it would be more flexible adding in the style and background props however I'm not sure it will be worth the time resolving breaking changes fixes esp because the plan is to convert to LumiComponent at some point in the near future, which would make these changes unnecessary. As we're trying to get this logo style variant up asap, I prefer just sticking with this for now (as it causes no breaking changes) and then working on the conversion to LumiComponent as it's own issue.

[arthurxavierx]

Ah yes, good point on converting to a LumiComponent! :)