-
Notifications
You must be signed in to change notification settings - Fork 41
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
Stub basic post type and API endpoint #6
Conversation
7dacd55
to
9b97f44
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.
CPT looks good (once I locally fixed the console warning with the keys) 👍
Since this isn't using wp-scripts, it's not running the linter, and there are lint issues in the JS. You should be able to copy the pattern-creator package.json entirely, then add this project as a workspace here, and it'll automatically work. Well, the JS file will need to move to src
, or you can pass --entry=./javascript/pattern-post-type.js
to wp-scripts, it'll still build to /build
. This can happen in this PR or a follow up.
* There's no need for webpack etc, since we can assume wp-admin users will have modern browsers. | ||
* | ||
* JSX is the only thing that'd be nice to have, but it's not worth the tooling costs for just a few fields. |
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.
Since we're already setting up the workspaces with a shared wp-scripts build process, I think it would be a better dev-experience to have this use it - we can let the build step handle pulling out dependencies, and will save us from needing to remember that one place uses a different syntax than others.
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 can see the appeal of everything using the same tooling, but it still feels to me like this doesn't need any tooling, and that the devex is better without me.
The existing tooling might might it easier to set this up, but it could still be a pain. The biggest cost IMO is using the tooling, though. It's slow, it's breaks often and is painful to fix, etc.
This is already working, too, so I don't see any benefit from redoing it now. If we want to add more fields or components later, then I can see that making the plugin complex enough where JSX would be worth the costs.
public_html/wp-content/plugins/pattern-directory/javascript/pattern-post-type.js
Show resolved
Hide resolved
public_html/wp-content/plugins/pattern-directory/javascript/pattern-post-type.js
Show resolved
Hide resolved
public_html/wp-content/plugins/pattern-directory/javascript/pattern-post-type.js
Show resolved
Hide resolved
public_html/wp-content/plugins/pattern-directory/javascript/pattern-post-type.js
Show resolved
Hide resolved
public_html/wp-content/plugins/pattern-directory/includes/pattern-post-type.php
Outdated
Show resolved
Hide resolved
What was that? |
Oh, you were getting that typical warnings about missing keys. Huh, that's weird that I didn't. Oh well 🤷🏻♂️ |
a81ed14
to
1a78a14
Compare
I'm gonna merge this since I'm leaving on AFK soon, but feel free to open an issue about JSX, or just make the change if you feel strongly. |
This creates a basic CPT, and tweaks Core's REST API endpoints to support listing and searching patterns.
Example API requests:
Fixes #4