-
Notifications
You must be signed in to change notification settings - Fork 15
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: Added cssClassName attribute to CoreGroup #328
base: main
Are you sure you want to change the base?
feat: Added cssClassName attribute to CoreGroup #328
Conversation
🦋 Changeset detectedLatest commit: c0528c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
… between WP versions slightly
/** | ||
* Test that the CoreGroup block is retrieved correctly. | ||
* | ||
* Covers the following attributes: |
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.
(this isn't a list of attributes.
Not saying you need one - we added it to track what Core*Attributes are actually being tested since this project doesn't have accurate codecov )
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.
Thanks @justlevine
I will update the comment and that's good to know about the code coverage.
Also thanks for the code review 👍
Co-authored-by: Dovid Levine <[email protected]>
Heads up I'm hoping to rip out this entire antipattern in rtCamp#31. If we want to diverge from WP as a source of truth, we shouldn't have those fields masquerade as block.json attributes and this has been a consistent source of conflicts when upgrading wp versions. Once a WPGraphQL Model is in place (so every new attribute isnt a perf drain due to |
Thanks @justlevine for the heads up and also the context around the upgrades. I can definitely see how this might cause issues with upgrades. Let me know if you want me to do any testing for your branch once your ready 👍 |
Overview
This adds the cssClassName for the CoreGroupAttributes as per issue reported here #327
Description
See above.
Related Issue(s):
#327
Testing
The following test checks that cssClassName is availble for the CoreGroupAttributes
Setup
If you update a page with the following HTML
If you then run the following query in WPGraphQL IDE
Note: Add the following query variables (replacing your page/post id)
You should get the following response
Note that the cssClassName for the CoreGroup attribute node is
wp-block-group test-group-class is-style-default is-content-justification-center is-layout-flex wp-container-core-group-is-layout-7 wp-block-group-is-layout-flex
Screenshots