-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix bugs in WordPress 5.8 related to withColors and modernize code in blocks #369
base: feature/wp-scripts-restructure
Are you sure you want to change the base?
Fix bugs in WordPress 5.8 related to withColors and modernize code in blocks #369
Conversation
… to `clip-path` outputs the attribute on the frontend as expected.
…r, remove 4.9 fallback code
…e for inserter, remove 4.9 fallback code
…5.8. Also additional modernization updates where possible.
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.
const getColorSlug = ( color ) => { ... }; is repeated at least 7 times, let's pull this out into a common library.
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.
yeah, I was contemplating this. I think I can add it to the ThemeOptions component but I'll have to modernize that component file too. I can make that change to the PR.
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 becomes an imported component?
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.
Everything looks good. I was able to run npm run build
with no errors (but lots of warnings). A quick check of the editor and frontend - everything appears to be working.
I did not do any VRTs against the current version of BU Blocks.
Other things to note:
npm audit
produces 19 vulnerabilities (6 low, 3 moderate, 10 high)npm run lint:js
produces 229 problems (226 errors, 3 warnings)
className
.large
size instead. This should be a safe change for most blocks but may result in some layout bugs. Loading thefull
size however is not acceptable as the files sizes can be extreme.