Skip to content
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

#6945: add a props block to sync mentions to wp.org #400

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

gaambo
Copy link

@gaambo gaambo commented May 17, 2023

Adds a "props" block as mentioned in #6945.

How

  • Adds a new block wporg/props which is based on the core paragraph block (and similar to the notice block already existing in this repo).
  • Block supports are copied from the core paragraph as of Gutenberg version 15.1 which includes text align, colors, typograph and spacing controls
  • On save_post it checks for existance of this block (also supports nested blocks) and parses all mentions in this props block.
  • The props are then sent to WordPress.org via WordPressdotorg\Profiles\add_activity.
  • Synced props are stored as post meta and on updating a post they are compared and only new props are synced.
  • It supports multiple props blocks in a post and also props blocks nested in others (like groups).
  • It supports all post types atm.
  • I've added the classnames dependency since its used in gutenberg and it's my go-to. But wasn't added in this package until yet and I don't think the DependencyExtractionPlugin of core handles that package. If we want to avoid this package for the simple two lines in this block, I can remove that.

Questions:

  • How can I test the add_activity call? Is there a test instance or a local instance I can get when running via wp-env?
  • Which values does the type parameter of add_activity allow?
  • I'm assuming existing Make blogs use bbPress and therefore bbp_find_mentions to parse mentions. I'm reusing this function and provide a fallback if this is not available. If bbPress and this function will always be available, we can remove the fallback.
  • It does not handle removing props (because I don't know if there's a remove_activity function?
  • I'm using the generic save_post hook here but if we only want to support the post post type, I can switch that to save_post_post.

How to test:

  1. Checkout this PR and run npm start + npm run wp-env start
  2. Create a new post
  3. Check the "Props" block is available in the inserter
  4. Enter some text in the block like "Props to @xy"
  5. You can add multiple props blocks and also nest them in others (like a group)
  6. Save the post
  7. ‼️ Atm this will trigger an error because on local environments WordPressdotorg\Profiles\add_activity does not exist
  8. You can comment out this code, click update again and check that the meta field _wporg_props does exist and has the correct values in it

Screenshots:

grafik

@gaambo
Copy link
Author

gaambo commented May 17, 2023

I've used npm run lint:js locally but don't get the error messages that the CI shows me.
Also I didn't commit my package-lock.json but classnames is in package.json so the CI build should get that?

@gaambo gaambo closed this Dec 1, 2023
@StevenDufresne
Copy link
Contributor

This PR is closed. Is that correct? I'm under the impression that it should still be open.

@@ -64,5 +64,7 @@
"selector-class-pattern": null
}
},
"dependencies": {}
"dependencies": {
"classnames": "^2.3.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid importing classnames. It don't think we need it for the block.

@dd32 dd32 reopened this Feb 21, 2024
@gaambo
Copy link
Author

gaambo commented Feb 21, 2024

This PR is closed. Is that correct? I'm under the impression that it should still be open.

@StevenDufresne
I didn't get any feedback or information if that block is still needed, or if I took the right direction. Also since the last commit the block API has changed and probably a few changes to the block should be made to work perfectly with the newest core versions. Since I had no time (and currently have no time) to work on this, I closed it.
Feel free to take it as-is and build on top of it.

@StevenDufresne
Copy link
Contributor

👍 Okay. I've added this to my list of things :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants