-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add unsupported block title translation test #33340
Add unsupported block title translation test #33340
Conversation
packages/block-library/src/missing/test/edit-integration.native.js
Outdated
Show resolved
Hide resolved
Size Change: +4.98 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
packages/block-library/src/missing/test/edit-integration.native.js
Outdated
Show resolved
Hide resolved
Continuing with this comment and since I have some context about the translations due to my work in the translation pipeline improvements, I played around with the i18n package and I think I found a reliable way to test this. Here is the patch of the changes I did, feel free to edit it: diff --git a/packages/block-library/src/missing/test/edit-integration.native.js b/packages/block-library/src/missing/test/edit-integration.native.js
index 4ffa2f8bf0f3945a052f6f83e6d5bc8ac91da1f2..0b2c0d12576182b79eadc6ac599d094434f7818e 100644
--- a/packages/block-library/src/missing/test/edit-integration.native.js
+++ b/packages/block-library/src/missing/test/edit-integration.native.js
@@ -1,20 +1,13 @@
/**
* External dependencies
*/
-import { initializeEditor } from 'test/helpers';
+import { initializeEditor, waitFor, within } from 'test/helpers';
/**
* WordPress dependencies
*/
import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks';
-import * as i18n from '@wordpress/i18n';
-jest.mock( '@wordpress/i18n', () => {
- const actual = jest.requireActual( '@wordpress/i18n' );
- return {
- ...actual,
- _x: jest.fn( actual._x ),
- };
-} );
+import { setLocaleData } from '@wordpress/i18n';
/**
* Internal dependencies
@@ -22,6 +15,8 @@ jest.mock( '@wordpress/i18n', () => {
import { registerCoreBlocks } from '../..';
beforeAll( () => {
+ setLocaleData( { 'block title\u0004Table': [ 'Tabla' ] } );
+
// Register all core blocks
registerCoreBlocks();
} );
@@ -31,6 +26,9 @@ afterAll( () => {
getBlockTypes().forEach( ( block ) => {
unregisterBlockType( block.name );
} );
+
+ // Clean up translations
+ setLocaleData( {} );
} );
describe( 'Unsupported block', () => {
@@ -38,13 +36,18 @@ describe( 'Unsupported block', () => {
const initialHtml = `<!-- wp:table -->
<figure class="wp-block-table"><table><tbody><tr><td>1</td><td>2</td></tr><tr><td>3</td><td>4</td></tr></tbody></table></figure>
<!-- /wp:table -->`;
- await initializeEditor( {
+ const { getByA11yLabel } = await initializeEditor( {
initialHtml,
} );
- // jest spy for the _x translation function
- const _xSpy = jest.spyOn( i18n, '_x' );
+ const missingBlock = await waitFor( () =>
+ getByA11yLabel( /Unsupported Block\. Row 1/ )
+ );
+
+ const translatedTableTitle = within( missingBlock ).getByText(
+ 'Tabla'
+ );
- expect( _xSpy ).toHaveBeenCalled();
+ expect( translatedTableTitle ).toBeDefined();
} );
} ); Setting translationsFor this purpose, I'm using the function gutenberg/packages/react-native-editor/src/index.js Lines 121 to 141 in c26c506
Block title localization keyThe key I used is
|
- Instead of checking if the `_x` translation function was called, we now instead mock the translations and verify that the translated string appears in the UI. - Add a test to verify translations in the unsupported block bottom sheet title.
Thank you so much for the detailed response here, @fluiddot! The approach you suggest is great and I've applied the patch to the code and updated the PR. I've also added another test to verify that the title e.g. "'Table' is not fully-supported" of the bottom sheet that's shown when the user taps on the help icon is translated. I'm marking this as ready for review and re-requesting a review from you 🙇. |
BTW @fluiddot – I created a new file here, |
It makes sense to me to have them separated but I'm wondering if eventually the original test (which is using the testing library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎊 !
I only added a comment with a suggestion but feel free to skip it as it's minor.
packages/block-library/src/missing/test/edit-integration.native.js
Outdated
Show resolved
Hide resolved
Thanks! Good idea, I made the change 👍 |
Description
Adds a test to the unsupported block placeholder (a.k.a
missing
) on mobile to ensure the block title is translated when an unsupported (core) block is rendered in the editor. Fixes wordpress-mobile/gutenberg-mobile#3669.How has this been tested?
git revert <merge_commit>
(conflicts in the changelog can be marked resolved as-is, since they'll be discarded later).gutenberg-mobile
(ensuring the changes from Step 1 are present in the Gutenberg submodule), changing the device language to a language other than English, opening the editor from within WPiOS and observing that unsupported blocks do not show their translated block name (they show a blank space instead).git revert
(git reset --hard <previous_commit>
).Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).