Update: Set property to change one property accept all lodash paths. #25159
Conversation
jorgefilipecosta
added
the
Global Styles
|
Size Change: +246 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
jorgefilipecosta
force-pushed
the
update/setProperty-to-change-one-property-and-accept-all-lodash-paths
branch
from
5240b97 to
c35c544
Compare
|
This PR seems to fix the custom gradient problem, so I enabled them again. |
There was a problem hiding this comment.
This is so great to see that I went ahead and pushed a couple of minor tweaks, so we can expedite this and merge as soon as CI gives us a 🟢
To be honest, I tried this same approach and it didn't work (obviously it wasn't the same, otherwise it will work). I didn't like the promise trick to coordinate gradient/background setters but failed to make it work without it (the two consecutive setters will interrupt each other and the gradients panel didn't work). I'm very happy to see this fixed!
This is a surprise to me. We do use string paths in a few places at the moment and there's no rule for this (although there was one a couple of years ago, but can't find when/why it was removed). The comment about adding the rule reported that the performance gains were minimal. I'm not too opinionated about one form or another, but if committing to one makes things consistent through the codebase, I'm game for that change. |
jorgefilipecosta
deleted the
update/setProperty-to-change-one-property-and-accept-all-lodash-paths
branch
I was not aware the rule was removed and I thought we still did not use string paths. Anyway, the change made does not block string paths, it makes it possible to use any path format, while before only string paths worked. |
When trying to address #25145 (comment), I noticed our setProperty global styles methods do not accept every lodash path but only string paths.
Previously we discovered that using string paths is slow and we have a lint rule that disallows string paths on lodash e.g:
get ( object, 'a.b' )is not accepted by our lint rules.I guess we should use array paths like the rest of the codebase using array paths would also allow us to use the global mappings that are in an array format.
The setProperty method allows changing multiple properties, each property is a key in the object, we can not use keys that are arrays. So this PR changes setProperty to change a single property at a time.
How has this been tested?
I verified the global styles sidebar continues to work.