Try: Override callout shortcodes in favor of the notice block

[ryelle]

Closes WordPress/wporg-developer#315

Remove the existing "callout" shortcodes, and replace them with the Notice block.

Some of the shortcode content can be very long

This shortcode is also used on classic themes, since it's part of the handbook code. I've updated the CSS to include fallbacks for the custom properties, so it should still work. For example, on make/core's handbook.

Is the sanitization + fitlering necessary? I grabbed it from the existing callout shortcode.

[adamwoodnz]

Tried this in sandbox on https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/ and it breaks quite badly. I'm not sure what exactly, but seems to be do with content parsing. The page is very slow to load and looks like this when fully loaded:

[pkevan]

@adamwoodnz I had a quick look at this, and it appears that the info shortcode gets into some kind of loop, when you remove it from the foreach the page renders quickly, so probably some bug in there.

[renintw]

Is the sanitization + fitlering necessary? I grabbed it from the existing callout shortcode.

I think the sanitization is still needed as the shortcode accepts users' input, but not so sure about the filter.

So I've checked a bit if o2 is activated on wp.org, make.wp.org, and developer.wp.org, and only make.wp.org has o2 cross-Posting Access network-activated; no other o2-related plugins are activated on any of these three sites (search with 'o2'). Does that mean we can remove the filter safely?

@dd32 I noticed that you have made several commits in the o2 plugin, so perhaps you have some thoughts on this. 🙂

[pkevan]

Does that mean we can remove the filter safely?

Do we envisage this not being used on make.wp.org? Given it's a mu-plugin, it's probably best to keep the filters in there, since it would automatically be loaded anyway. You'd ideally check the occurrence of the legacy shortcodes too.

[renintw]

The root cause is this line.
This filter would execute all functions hooked onto it and then formats $content, it ends up adding various extra tags, \n, etc. Something among them seems to cause do_blocks to get into a loop.

Removing this filter directly resolves the issue. (i.e., comment out these three lines)

Additionally, it also needs to remove the <p> tags from within the shortcode content, as Gutenberg interprets them as blank lines. PHP: PHPUnit ⬇️

With <p> tag	Without <p> tag + Add a newline

Adv Admin landing page works correctly.

@adamwoodnz I'll run a few more tests based on this tomorrow to see if there are no issues. It'd be nice if you could also take a look. The scope of this change seems quite extensive, it's great to have extra eyes to help check.

[renintw]

Do we envisage this not being used on make.wp.org

The filter seems there for the plugin o2, but it doesn't seem to be activated anymore.

You'd ideally check the occurrence of the legacy shortcodes too.

Probably a dumb question. What does a legacy shortcode refer to there, any examples?

But since the_content seems to be the root cause here, once it's removed, o2_process_the_content filter is no longer required.

[pkevan]

What does a legacy shortcode refer to there, any examples?

The ones being replaced by this code is what I was attempting to refer to 😄 [ 'info', 'tip', 'alert', 'tutorial', 'warning' ]

[renintw]

I realized that we cannot remove the 'the_content' filter, as this would lead to losing some features like auto-linking or small features like tooltips.

Since earlier I placed the same content that's broken on production locally and it did not break, I guess that the problem might be on $content. After being processed by do_blocks, it is not compatible with the traditional theme of make.wp.org.

So eventually I just move $content outside the do_blocks to prevent it from being processed by the function.
I've tested several pages on both Make and Developer sites while sandboxed, and it seems like there were no issues.

I guess the remaining task is first to convert the content that contains <div class="callout callout-warning"> into shortcodes. Both make.wp.org and developer.wp.org still have content using this format.
Then include @ndiego's idea of extending the logic here to cover the GitHub markdown.

Reference: WordPress/wporg-developer#315 (comment)

[adamwoodnz]

Created a test post https://developer.wordpress.org/redesign-test/2023/12/12/callouts/

Screenshots

Before	After

[adamwoodnz]

Current Developer pages look ok

[adamwoodnz]

New developer pages look good

[adamwoodnz]

Tried this in sandbox on https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/ and it breaks quite badly.

This page is fixed, apart from the new line issue 🎉

@renintw what do you propose for the empty p tags, does this involve a lot of handbook editing?

Some have the issue, others don't

[ndiego]

Colors and icons look great! Can the links be in blueberry and a border-radius of 2px be applied? The Figma design did not show links (that I can find), but we use blueberry in other similar content areas.

[adamwoodnz]

There's an alert on https://make.wordpress.org/core/handbook/about/release-cycle/releasing-major-versions/ which isn't being captured for some reason

The alert on https://make.wordpress.org/core/handbook/about/organization/ is ok though

Trying some other handbooks and found https://make.wordpress.org/test/handbook/test-reports/ has on old style info box too

[adamwoodnz]

There's an alert on https://make.wordpress.org/core/handbook/about/release-cycle/releasing-major-versions/ which isn't being captured for some reason

Trying some other handbooks and found https://make.wordpress.org/test/handbook/test-reports/ has on old style info box too

Turns out these are both using <div class="callout"... rather than shortcodes, the same as the Block Editor handbook does.

I think we'll need to do a secondary PR to update styles for these in the Developer repo, as a short term fix before we add support for the GitHub markdown approach previously discussed.

For these Make sites I'm not sure how concerned we should be about the inconsistency if we merge this. I don't think it's a major. Thoughts @ndiego @renintw ?

[adamwoodnz]

Works for me; tested on old and new Developer sites, Make Core, Make Test and Make Meta. One question above regarding inconsistencies with handbooks using callout markup rather than shortcodes.

[ndiego]

Works for me; tested on old and new Developer sites, Make Core, Make Test and Make Meta. One question above regarding inconsistencies with handbooks using callout markup rather than shortcodes.

Will all the Developer sites be consistent?

[adamwoodnz]

Will all the Developer sites be consistent?

Do you mean all the callouts/shortcodes on the Developer site? Not without further work for the Block Editor Handbook. I think our short term options are:

1. Add some styles for the existing .callout markup to either the parent theme (if we think this will stick around, or the Documentation site also uses this) or the Developer theme
2. Migrate the Block Editor Handbook callout markup to shortcodes to take advantage of this PR.

I think I prefer 1 as 2 feels like a backward step.

Medium term I think we should push forward with the plan of using GitHub markdown and coverting that to Notice blocks.

[adamwoodnz]

1. Add some styles for the existing .callout markup to either the parent theme (if we think this will stick around, or the Documentation site also uses this) or the Developer theme

PR raised: WordPress/wporg-developer#429

[renintw]

@renintw what do you propose for the empty p tags, does this involve a lot of handbook editing?

It doesn't need a lot of editing on a single handbook post, but it would need a lot on all the posts.
It simply needs to remove the <p> tags from within the shortcode content, as Gutenberg interprets them as blank lines. PHP: PHPUnit ⬇️

With <p> tag	Without <p> tag + Add a newline

We can write a regex to replace all the p tags inside the shortcode, and also starting from the second p tag, in addition to getting rid of it, we also need to add a new line before it.
But I would more prefer to possibly share with the community, asking folks to see and report back, and then make modifications.

[renintw]

I think I prefer 1 as 2 feels like a backward step.

Thanks for opening the PR for it, 1 does make more sense than 2.

[renintw]

Colors and icons look great! Can the links be in blueberry and a border-radius of 2px be applied? The Figma design did not show links (that I can find), but we use blueberry in other similar content areas.

I've changed the border-radius to 2px.

For links color, they should've been blueberry, but somehow the styles here are applied, even though those actions have not been performed on links. Removing those pseudo-classes does make it render blueberry.

Do you run into this as well @adamwoodnz?

[adamwoodnz]

Looks like we should change the notice font size to --wp--preset--font-size--small too, based on this comment and the designs.

[adamwoodnz]

For links color, they should've been blueberry, but somehow the styles here are applied, even though those actions have not been performed on links. Removing those pseudo-classes does make it render blueberry.

Do you run into this as well @adamwoodnz?

Yep same here. Shall we add a new --wp--custom--wporg-notice--color--link var and update those link styles then?

[adamwoodnz]

2 suggestions inline then I think we can merge this 👍

[renintw]

Yep same here. Shall we add a new --wp--custom--wporg-notice--color--link var and update those link styles then?

Huh, I see the problem now. It shouldn't have added a:link, as it conflicts with a:visited. The former applies to all unvisited hyperlinks, while the latter is for links that have already been visited.
I think I'll just remove all actions here, unless we need to change the text to black after an action, which I don't see in the design.