Conversation
There was a problem hiding this comment.
Oh, this is clever, thanks for the follow-up @ciampo! The other use case that we will have for the Box type that isn't covered yet is when we add in position support, where the lower case top, right, bottom, and left will be used directly. Is there a way to have those words be lowercase if BoxVariants is an empty string?
(For reference, here's an early WIP PR looking at adding in position support — it'll likely change a lot, but just for context as to why it was on my mind 😀)
|
You raise a good point! I went ahead and allowed I quickly tested it in my IDE and it seems to work as expected (note that box-variants-ts.mp4 |
andrewserong
left a comment
There was a problem hiding this comment.
Nice one, thanks @ciampo — that's looking good to me now, and confirmed that it's working the same in my IDE as it is in your screengrab 👍
Looks like there's a failing e2e unrelated to this PR, so it might need either a rebase or to kick off the tests again before you merge it in.
Thanks again for the follow-up!
7dd7818 to
fa85133
Compare
Description
Follow-up to this conversation, this PR proposes a more advanced type for
Boxwhich is closer to the actual values thatBoxis going to assume.Testing Instructions
Screenshots
Types of changes
Enhancement
Checklist:
*.native.jsfiles for terms that need renaming or removal).