-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Enable applying of gradients and pattern on line segments #11217
Conversation
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 think it should still return false if prevStyle is undefined. Also might be worth comparing the object instances for equality when its pattern or gradient
You're right! I'll do
Ok, therefore scans the object properties checking if the values are equals. right? |
Thats what I was thinking. Maybe its ok to just check each property and not use JSON.stringify at all.. |
Fully agree. I'm busy now in a couple of other things. As soon as I'll finish, I'll change it and re-commit. I was checking before if there is already a helpers function which is doing that but I don't think so. |
it sounds more complex than I expected... Let me propose another options using stringify. |
if (!cache.includes(value)) { | ||
cache.push(value); | ||
} | ||
return cache.indexOf(value); |
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.
A map could be more efficient in these. But I'm not really worried about that. My main worry is a confusion between index in the array and some other possible numeric value, which is probably a non-issue really.
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 was thinking to use a map but the array could contain max 4 objects.
Not clear about the confusion. The index is used only to compare a gradient/pattern. We needed a unique value per object instance to put in the json. It seemed to me better than a string or object.
Fix #11204