try fixing that v1 markup on published pages #2290
Conversation
|
Download coblocks.zip: https://46451-128991767-gh.circle-artifacts.com/0/tmp/artifacts/coblocks-2290.zip |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
AnthonyLedesma
requested review from
jrtashjian,
EvanHerman,
jonathanbardo and
olafleur-godaddy
as code owners
| * @return boolean True when post content contains a v1 Masonry block. | ||
| */ | ||
| public function has_masonry_v1_block() { | ||
| $v1_regex = '/<!-- wp:coblocks\/gallery-masonry.*|\n*(coblocks-gallery--item).*|\n*<!-- \/wp:coblocks\/gallery-masonry -->/m'; |
There was a problem hiding this comment.
You shouldn't need the multi-line flag (/m) here because you're not using the beginning string (^) and end of string ($) modifiers. Just the slash without any flags would work.
There was a problem hiding this comment.
Posting here for reference a possible new regex so we can simplify the function: https://regex101.com/r/xESuJM/1
/<!--\s+?wp:coblocks\/gallery-masonry[\s\S]*?coblocks-gallery--item/
There was a problem hiding this comment.
This would also work:
/<!--\s+?wp:coblocks\/gallery-masonry.*?coblocks-gallery--item/s
| preg_match_all( $v1_regex, get_the_content(), $matches ); | ||
| return isset( $matches[0] ) && isset( $matches[0][2] ) && ! empty( $matches[0][2] ); |
There was a problem hiding this comment.
We don't need to store and perform conditions on any of the matches, we just want to know if there is a match. This can be simplified with:
return preg_match( $v1_regex, get_the_content() );
|
We ended up coming across a few issues and edge cases with regex that will adequately match the V1 masonry block. There is one known edge case that will cause users to superfluously load Masonry/jQuery scripts. Edge Case
|
AnthonyLedesma
dismissed
jrtashjian’s stale review
Deciding to keep changes after discussion
Description
In cases where users have old V1 masonry markup already saved and on published pages we have broken galleries. This fallback will allow for the old Masonry markup to continue to function as expected.
Note
We ended up coming across a few issues and edge cases with regex that will adequately match the V1 masonry block.
There is one known edge case that will cause users to superfluously load Masonry/jQuery scripts.
Edge Case
If a user has V2 Masonry > (Offset Gallery OR Carousel Gallery) > V2 Masonry.
Screenshots
Before
After
Types of changes
Re-Introduce jQuery as a fallback :(
How has this been tested?
Tested manually.