[RNMobile] Fix Gallery v1 caption selector #51076
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +1 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 2fc5e10. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5117454671
|
derekblank
added
Mobile App - i.e. Android or iOS
derekblank
changed the title
4 tasks
|
I was able to reproduce the original issue in the demo app with the following: Demo app with v1 gallery patchdiff --git a/packages/edit-post/src/editor.native.js b/packages/edit-post/src/editor.native.js
index b031601186..63ec4451ca 100644
--- a/packages/edit-post/src/editor.native.js
+++ b/packages/edit-post/src/editor.native.js
@@ -32,7 +32,7 @@ class Editor extends Component {
// need to set this globally to avoid race with deprecations
// defaulting to true to avoid issues with a not-yet-cached value
- const { galleryWithImageBlocks = true } = props;
+ const { galleryWithImageBlocks = false } = props;
window.wp.galleryBlockV2Enabled = galleryWithImageBlocks;
if ( props.initialHtmlModeEnabled && props.mode === 'visual' ) {
diff --git a/packages/react-native-editor/src/initial-html.js b/packages/react-native-editor/src/initial-html.js
index 9cf963fa7b..de9c1f6e62 100644
--- a/packages/react-native-editor/src/initial-html.js
+++ b/packages/react-native-editor/src/initial-html.js
@@ -1,236 +1,3 @@
-export const textBlocks = `<!-- wp:heading -->
-<h2 class="wp-block-heading" id="this-is-an-anchor">What is Gutenberg?</h2>
-<!-- /wp:heading -->
-
-<!-- wp:paragraph -->
-<p><strong>Bold</strong> <em>Italic</em> <s>Striked</s> Superscript<sup>(1)</sup> Subscript<sub>(2)</sub> <a href="http://www.wordpress.org" target="_blank" rel="noreferrer noopener">Link</a></p>
-<!-- /wp:paragraph -->
-
-<!-- wp:heading {"textAlign":"left","level":4,"className":"has-primary-background-color has-background","style":{"typography":{"lineHeight":"2.5"}}} -->
-<h4 class="wp-block-heading has-text-align-left has-primary-background-color has-background" style="line-height:2.5">Heading with line-height set</h4>
-<!-- /wp:heading -->
-
-<!-- wp:list -->
-<ul><!-- wp:list-item -->
-<li>First Item</li>
-<!-- /wp:list-item -->
-
-<!-- wp:list-item -->
-<li>Second Item</li>
-<!-- /wp:list-item -->
-
-<!-- wp:list-item -->
-<li>Third Item</li>
-<!-- /wp:list-item --></ul>
-<!-- /wp:list -->
-
-<!-- wp:quote {"align":"left","className":"is-style-large"} -->
-<blockquote class="wp-block-quote has-text-align-left is-style-large"><!-- wp:paragraph -->
-<p>"This will make running your own blog a viable alternative again."</p>
-<!-- /wp:paragraph --><cite>— <a href="https://twitter.com/azumbrunnen_/status/1019347243084800005">Adrian Zumbrunnen</a></cite></blockquote>
-<!-- /wp:quote -->
-
-<!-- wp:pullquote -->
-<figure class="wp-block-pullquote"><blockquote><p>One of the hardest things to do in technology is disrupt yourself.</p><cite>Matt Mullenweg</cite></blockquote></figure>
-<!-- /wp:pullquote -->
-
-<!-- wp:paragraph {"dropCap":true,"className":"custom-class-1 custom-class-2 has-background has-vivid-red-background-color","fontSize":"large"} -->
-<p class="has-drop-cap custom-class-1 custom-class-2 has-background has-vivid-red-background-color has-large-font-size">
-Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer tempor tincidunt sapien, quis dictum orci sollicitudin quis. Proin sed elit id est pulvinar feugiat vitae eget dolor. Lorem ipsum dolor sit amet, consectetur adipiscing elit.</p>
-<!-- /wp:paragraph -->
-
-<!-- wp:preformatted -->
-<pre class="wp-block-preformatted">Some <em>preformatted</em> text...<br>And more!</pre>
-<!-- /wp:preformatted -->
-
-<!-- wp:code -->
-<pre class="wp-block-code"><code>if name == "World":
- return "Hello World"
-else:
- return "Hello Pony"</code></pre>
-<!-- /wp:code -->
-
-<!-- wp:verse {"textAlign":"center"} -->
-<pre class="wp-block-verse has-text-align-center">Come<br>Home.</pre>
-<!-- /wp:verse -->`;
-
-export const mediaBlocks = `<!-- wp:image -->
-<figure class="wp-block-image"><img alt=""/></figure>
-<!-- /wp:image -->
-
-<!-- wp:image -->
-<figure class="wp-block-image"><img src="https://cldup.com/cXyG__fTLN.jpg" alt=""/><figcaption class="wp-element-caption">Mountain</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:video {"id":683} -->
-<figure class="wp-block-video"><video controls src="https://i.cloudup.com/YtZFJbuQCE.mov"></video><figcaption class="wp-element-caption">Videos too!</figcaption></figure>
-<!-- /wp:video -->
-
-<!-- wp:file /-->
-
-<!-- wp:file {"id":3,"href":"https://wordpress.org/latest.zip"} -->
-<div class="wp-block-file"><a href="https://wordpress.org/latest.zip">WordPress.zip</a><a href="https://wordpress.org/latest.zip" class="wp-block-file__button wp-element-button" download>Download</a></div>
-<!-- /wp:file -->
-
-<!-- wp:audio /-->
-
-<!-- wp:audio {"id":5} -->
-<figure class="wp-block-audio"><audio controls src="https://cldup.com/59IrU0WJtq.mp3"></audio></figure>
-<!-- /wp:audio -->
-
-<!-- wp:gallery {"columns":8,"linkTo":"none","className":"alignfull"} -->
-<figure class="wp-block-gallery has-nested-images columns-8 is-cropped alignfull"><!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon.png" alt=""/><figcaption class="wp-element-caption">Paragraph</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Heading.png" alt=""/><figcaption class="wp-element-caption">Heading</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Subheading.png" alt=""/><figcaption class="wp-element-caption">Subheading</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Quote.png" alt=""/><figcaption class="wp-element-caption">Quote</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Image.png" alt=""/><figcaption class="wp-element-caption">Image</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Gallery.png" alt=""/><figcaption class="wp-element-caption">Gallery</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Cover-Image.png" alt=""/><figcaption class="wp-element-caption">Cover Image</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Video.png" alt=""/><figcaption class="wp-element-caption">Video</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Audio.png" alt=""/><figcaption class="wp-element-caption">Audio</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Column.png" alt=""/><figcaption class="wp-element-caption">Columns</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-File.png" alt=""/><figcaption class="wp-element-caption">File</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Code.png" alt=""/><figcaption class="wp-element-caption">Code</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-List.png" alt=""/><figcaption class="wp-element-caption">List</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Button.png" alt=""/><figcaption class="wp-element-caption">Button</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-Embeds.png" alt=""/><figcaption class="wp-element-caption">Embeds</figcaption></figure>
-<!-- /wp:image -->
-
-<!-- wp:image {"sizeSlug":"large","linkDestination":"none"} -->
-<figure class="wp-block-image size-large"><img src="https://wordpress.org/gutenberg/files/2018/07/Block-Icon-More.png" alt=""/><figcaption class="wp-element-caption">More</figcaption></figure>
-<!-- /wp:image --></figure>
-<!-- /wp:gallery -->
-
-<!-- wp:media-text {"isStackedOnMobile":false,"className":"is-stacked-on-mobile"} -->
-<div class="wp-block-media-text alignwide is-stacked-on-mobile"><figure class="wp-block-media-text__media"></figure><div class="wp-block-media-text__content"><!-- wp:paragraph {"className":"has-large-font-size"} -->
-<p class="has-large-font-size"></p>
-<!-- /wp:paragraph --></div></div>
-<!-- /wp:media-text -->
-
-<!-- wp:cover {"url":"https://cldup.com/cXyG__fTLN.jpg","id":890,"dimRatio":20,"overlayColor":"luminous-vivid-orange","focalPoint":{"x":"0.63","y":"0.83"},"minHeight":219} -->
-<div class="wp-block-cover" style="min-height:219px"><span aria-hidden="true" class="wp-block-cover__background has-luminous-vivid-orange-background-color has-background-dim-20 has-background-dim"></span><img class="wp-block-cover__image-background wp-image-890" alt="" src="https://cldup.com/cXyG__fTLN.jpg" style="object-position:63% 83%" data-object-fit="cover" data-object-position="63% 83%"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","className":"has-text-color has-very-light-gray-color","fontSize":"large"} -->
-<p class="has-text-align-center has-text-color has-very-light-gray-color has-large-font-size">Cool cover</p>
-<!-- /wp:paragraph --></div></div>
-<!-- /wp:cover -->
-`;
-
-export const otherBlocks = `
-<!-- wp:nextpage -->
-<!--nextpage-->
-<!-- /wp:nextpage -->
-
-<!-- wp:more -->
-<!--more-->
-<!-- /wp:more -->
-
-<!-- wp:spacer -->
-<div style="height:100px" aria-hidden="true" class="wp-block-spacer"></div>
-<!-- /wp:spacer -->
-
-<!-- wp:group -->
-<div id="this-is-another-anchor" class="wp-block-group"><!-- wp:paragraph -->
-<p>One.</p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph -->
-<p>Two</p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph -->
-<p>Three.</p>
-<!-- /wp:paragraph --></div>
-<!-- /wp:group -->
-
-<!-- wp:columns {"className":"gutenberg-landing\u002d\u002ddevelopers-columns has-2-columns"} -->
-<div class="wp-block-columns gutenberg-landing--developers-columns has-2-columns"><!-- wp:column -->
-<div class="wp-block-column"><!-- wp:paragraph {"align":"left"} -->
-<p class="has-text-align-left"><strong>Built with modern technology.</strong></p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph {"align":"left"} -->
-<p class="has-text-align-left">Gutenberg was developed on GitHub using the WordPress REST API, JavaScript, and React.</p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph {"align":"left","fontSize":"small"} -->
-<p class="has-text-align-left has-small-font-size"><a href="https://wordpress.org/gutenberg/handbook/language/">Learn more</a></p>
-<!-- /wp:paragraph --></div>
-<!-- /wp:column -->
-
-<!-- wp:column -->
-<div class="wp-block-column"><!-- wp:paragraph {"align":"left"} -->
-<p class="has-text-align-left"><strong>Designed for compatibility.</strong></p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph {"align":"left"} -->
-<p class="has-text-align-left">We recommend migrating features to blocks, but support for existing WordPress functionality remains. There will be transition paths for shortcodes, meta-boxes, and Custom Post Types.</p>
-<!-- /wp:paragraph -->
-
-<!-- wp:paragraph {"align":"left","fontSize":"small"} -->
-<p class="has-text-align-left has-small-font-size"><a href="https://wordpress.org/gutenberg/handbook/reference/faq/">Learn more</a></p>
-<!-- /wp:paragraph --></div>
-<!-- /wp:column --></div>
-<!-- /wp:columns -->
-
-<!-- wp:latest-posts {"displayPostContent":true,"displayPostDate":true} /-->
-
-<!-- wp:buttons -->
-<div class="wp-block-buttons"><!-- wp:button -->
-<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">Solid Button</a></div>
-<!-- /wp:button -->
-
-<!-- wp:button {"gradient":"luminous-vivid-amber-to-luminous-vivid-orange"} -->
-<div class="wp-block-button"><a class="wp-block-button__link has-luminous-vivid-amber-to-luminous-vivid-orange-gradient-background has-background wp-element-button">Gradient Button</a></div>
-<!-- /wp:button --></div>
-<!-- /wp:buttons -->
-
-<!-- wp:shortcode -->
-[youtube https://www.youtube.com/watch?v=ssfHW5lwFZg]
-<!-- /wp:shortcode -->
-
-<!-- wp:rss /-->
-`;
-
-export default textBlocks + mediaBlocks + otherBlocks;
+export default `<!-- wp:gallery {"ids":[null],"linkTo":"none","className":"columns-1"} -->
+<figure class="wp-block-gallery columns-1 is-cropped"><ul class="blocks-gallery-grid"><li class="blocks-gallery-item"><figure><img src="https://insufficient-octopus.jurassic.ninja/wp-content/uploads/2023/05/Phalacrocorax_carbo_Egretta_garzetta_and_Mareca_strepera_in_Taudha_Lake.jpg" alt="" data-id="9" data-full-url="https://insufficient-octopus.jurassic.ninja/wp-content/uploads/2023/05/Phalacrocorax_carbo_Egretta_garzetta_and_Mareca_strepera_in_Taudha_Lake.jpg" data-link="https://insufficient-octopus.jurassic.ninja/?attachment_id=9" class="wp-image-9"/><figcaption class="blocks-gallery-item__caption">test caption!</figcaption></figure></li></ul></figure>
+<!-- /wp:gallery -->`;And the change in this PR fixes the issue. While debugging, it seems that the truncated class name ( Oddly, this does not seem to occur on the web editor, though the implementation is shared. 🤔 Also, it is noteworthy that the markup in the web editor (code mode) reveals that the @scruffian 👋 🙂 , I saw that this class was added in this PR (along with the change that removed the From the mobile perspective, this PR fixes an issue for users still on the v1 gallery, but without a complete understanding of the web implementation, it'd be good to get feedback from the web perspective on it before landing it. Any clarity you can add is greatly appreciated. |
I tested using a site with WP 5.8 and Gutenberg plugin 13.6.0 (this version is where #41140 was released) and managed to reproduce the issue wordpress-mobile/WordPress-Android#17712 on the web too. When editing the caption of a gallery item, if I save the post and reload the editor, it results in the block being broken. gallery-block-image-caption.mp4 |
@mkevins AFAIK, the web version currently only supports Gallery v2 (reference) so this fix would only affect the native version as it's the only one where v1 is used. |
Good to know @fluiddot, thanks for pointing out that reference! Given that this change would only affect native, do you think we need to await any further feedback from the web team, or could we approve this PR? |
@derekblank From my side, I'd say we could proceed to finish this PR, as the change will only impact the native version. In any case, it would be great to have feedback from @scruffian in this regard, especially to double-check on the solution, and if in the future we run into a similar problem. That said, I'll proceed to review the PR and share potential feedback. |
fluiddot
approved these changes
Jun 2, 2023
There was a problem hiding this comment.
I checked that with these changes adding a caption doesn't break the block in the app. However, it does when opening the post in the web version.
web-app-gallery-block-caption.mp4
Ideally, we should also have this fix in the Gutenberg plugin but we can't since Gallery v1 was deprecated. Those users that still use Gallery v1 are likely to still experiment with this issue in the web version, but at least they won't in the app. Eventually, we'd need to consider removing v1 in the app and making the block unsupported. In the meantime, I think we could continue with this fix to improve the experience although, as I mentioned, it won't be perfect.
Regarding the original issue, I think it's caused by an incompatibility between the save function and the block schema, as they reference a different value:
gutenberg/packages/block-library/src/gallery/block.json
Lines 48 to 52 in 88d1e95
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes an issue on mobile with Gallery block v1 when a caption is added to an image. Gallery block v2 behavior is not affected.
H/T @mkevins for collaborating on this fix. 🎉
Why?
When a caption is added to an image in a v1 Gallery, the caption's class selector is
blocks-gallery-item, which causes the caption to be interpreted as a second gallery item on mobile, asblocks-gallery-itemis the selector for gallery items. The caption's class name should beblocks-gallery-item__caption, which correctly attributes the caption as a caption when parsed, and also matches the web behavior for the v1 Gallery block.Fixes:
galleryWithImageBlocksprop value based on the WordPress version of the site when value is unknown #47782How?
Updates the image caption class name from
block-gallery-itemtoblock-gallery-item__caption.Gallery block v1 HTML generated from web (hint: scroll to the end to note differences):
Gallery block v1 HTML generated from mobile (without the change included in this PR):
Gallery block v1 HTML generated from mobile (with the change included in this PR):
Note: the extra
wp-element-captionclass in the mobile examples is used for testing, referenced in the commit messages of #41140.It's also important to note that the web version of Gallery block v1 is using the correct caption selector,
block-gallery-item__caption. Without further investigation, it is unclear how this was handled in web, which apparently is bypassing the code that was changed in this file, but breaks in mobile. Perhaps @scruffian can provide more insight how the caption class is generated for Gallery block v1 web.Also to note: as this change only affects the deprecated Gallery block v1, what (if any) additional test coverage should we consider adding, besides manual testing?
Testing Instructions
figure+ul+li+image], and Gallery block v2 will use [figure+image].Screenshots or screencast
Not.Working.mov
Working.mov