-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Theme.json: Remove custom prefixes from properties that did not land in 5.8 #34485
Conversation
Size Change: +232 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
I'm thinking about what @mtias mentioned here. Perhaps we can build two back-compatibility bridges to cover most of the cases and give people time to adapt? I'm thinking of:
These bridges can be in place for a few Gutenberg versions or until we land this in WordPress 5.9. We should give a heads up in the Gutenberg release post or adding deprecations, etc. I reckon there're still ways in which this change forces 3rd parties to adapt: for example, if they're working directly with the cc @jorgefilipecosta for thoughts. |
I missed that comment, thanks.
Your suggestions work for me. I'm happy to put this PR on ice and close it after splitting out the minor bug fixes. |
Closing this PR as the minor bug fixes are in their own PRs now. |
Nice work on splitting the PR to land faster the bugfixes 👏 My comment wasn't meant to close this but to add those two things! My stance would be that those are enough given the experimental nature of the existing properties. It's also best to do it as far as possible from the integration cycle (we want to give people plenty of time to adapt instead of having to change things at the last minute). |
Ok, thanks, I jumped the gun closing this one then. I'll add the requested updates to useSetting along with a new theme.json schema class to map the custom prefixed properties over to their newer versions for v1. |
c0753f5
to
ad0bec5
Compare
I've rebased this PR to bring it up-to-date. The A new v1 schema class was added as an interim measure to migrate the prefixed flags to their nonprefixed version within the It's entirely possible I've missed something here so I'll give this further testing tomorrow. In the meantime, @oandregal can you confirm we still want these changes to land for 5.9 and the updates to |
The `customTextDecorations` and `customTextTransforms` properties needed to be mapped to the singular versions `textDecoration` and `textTransform` respectively in order to avoid conflicts with i18n of presets for them.
The customRadius flag did actually land in 5.8 and must be maintained until we switch to a full v2 of the theme.json schema.
ad0bec5
to
7efa5d6
Compare
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.
Nice, thank you!
Related: #31366 (comment), #31366 (comment)
Description
This PR removes the
custom
prefix from properties within theme.json that do not require it. Those that remain have already landed in WP 5.8 and will be addressed in the future by adding a migration fromv1
to the futurev2
theme.json schema.How has this been tested?
Testing instructions
false
.Example gist here
fontStyle
fontWeight
textDecorations
textTransforms
letterSpacing
custom
prefixed form e.g.customFontStyle
orcustomColor
. Then confirm the relevant controls still display correctly in both editors.useSetting
will return the correct value when using the deprecatedcustom
prefixed flag.Example patch to test `useSetting` with deprecated flags
Types of changes
Enhancement.
Breaking change ( 3rd parties working directly with theme.json or store shape will need updating )
Checklist:
*.native.js
files for terms that need renaming or removal).