feat: block-insertion - Githubissues

Automattic / super-cool-ad-inserter-plugin

Insert ads, newsletter signup widgets, and calls to action in the middle of your posts!

https://wordpress.org/plugins/super-cool-ad-inserter/#description

32 stars 4 forks source link

feat: block-insertion #65

Closed adekbadek closed 2 years ago

adekbadek commented 3 years ago

All Submissions:

[x] Have you followed the Newspack Contributing guideline?
[x] Does your code follow the WordPress' coding standards and VIP Go coding standards?
[x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes #54.

How to test the changes in this Pull Request:

Configure the plugin to use a couple of insertion positions, using the Widgets editor
Create two posts - one with block content, another with a single Classic block (as would be the case with pre-block-editor content)
Observe that on both posts the SCAIP insertions work as expected
Observe that a SCAIP insertion is always between top-level elements (that #54 is irreproducible)

Other information:

[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[ ] Have you written new tests for your changes, as applicable?
[x] Have you successfully ran tests with your changes locally?

adekbadek commented 3 years ago

Hi @benlk , could you have another look? 🙏

benlk commented 3 years ago

I will try to take a look this weekend. This will require creating a set of test post_contents that can be used for testing this plugin.

miguelpeixe commented 2 years ago

After some discussion and tests, the classic editor potential problem with nested elements is not common enough that we need to solve before releasing this update.

The other concerns are still valid, what do you think @adekbadek?

It should also skip <p> </p> by default, which is pretty common when copy+pasting from Google Docs.

The blacklisted blocks could also have apply_filters() on them. Different sites with different blocks plugins are likely going to use this. I also suggest using a different term for it: ignore_list is an idea.

adekbadek commented 2 years ago

The blacklisted blocks could also have apply_filters() on them. Different sites with different blocks plugins are likely going to use this. I also suggest using a different term for it: ignore_list is an idea.

Done in 084885b

It should also skip <p> </p> by default, which is pretty common when copy+pasting from Google Docs.

Done in d6533ee

matticbot commented 2 years ago

:tada: This PR is included in version 0.4.0-alpha.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

matticbot commented 2 years ago

:tada: This PR is included in version 0.4.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: