Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for boxShadow #6749
base: main
Are you sure you want to change the base?
feat: Add support for boxShadow #6749
Changes from 22 commits
6f85a87
2954f56
c8d4615
481c3ff
fd9d5da
a68aade
96296d8
3deb7bc
e3b5f76
dec4ba2
1e6d049
244e2c0
6f61bb9
cba386f
64a9e7a
6d05428
5fa603a
18259dc
af7a519
9bace78
e43a655
af634fc
deedd17
346a461
6bc6fec
456a2e3
3f7c69e
9f8bd41
b34a22d
2ab6687
6b3cfd3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of iterating over the entire prop object, let's iterate over
NestedColorProperties
and check if those properties exist in the prop object because the prop object can potentially have more fields than those listed inNestedColorProperties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prop object being the general object with all the props passed (backgroundColor, width, boxShadow etc.)? Because in current approach I use the loop that was there before (loop through keys in props), and if there is a boxShadow in props (boxShadow being an array by default), I iterate through objects in array (most cases one object), and check for if there is a color prop in boxShadow object
Can you elaborate more? Do you want to move checking the NestedProp outside the parent loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to copy other objects from the style, such as transforms? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this approach we also copy transform yes, but it doesn't affect it - it only makes sure any nested the color property will work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe to avoid potential performance degradation, let's check if this property is specifically a box-shadow. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this line the original
key
value is0
rather thatboxShadow
, so we cannot use it. It is because we are doing:I would suggest saving initial
key
tocurrentKey
prop, and passing it. It supports your idea of avoiding performance degradation as well as it's not invasive.by default, on the first run,
runAnimations
function doesn't haveoriginalKey
socurrentKey
becomeskey
. But when we go deeper i.ex. run animation for each item of the array (like we do inboxShadow
case), thecurrentKey
stays asboxShadow
(or any other propName) and lets us determine if we want to spread theanimation.current
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can push this changes so you can see first hand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updates[propName]
is an array?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toValue
andcurrent
(in other words it made sure the animation wouldn't jump)