fix: return createBlock instances from block transforms and fix filter CSS selectors
Two related bug fixes: - Transforms: `type: 'block'` callbacks must return a block instance created by createBlock; the previous code returned raw attribute objects which WordPress silently ignored, so the toolbar block-switcher transforms never fired. Update tests to assert on `result.name` and `result.attributes`. Add a manual jest mock for @wordpress/blocks since the package is provided at runtime via dependency-extraction-webpack-plugin. - Filter CSS: the SCSS used a descendant combinator (`.wp-block-ksolo-image-filter .has-filter-warm img`) but the JSX puts both classes on the same element, so the filter rule never matched and picking a filter had no visual effect on the editor or the front end. Target the inner <img> by its stable class pair (`.ksolo-image-filter-img.has-filter-<slug>`) instead. Add a regression test that reads the compiled CSS and asserts the selectors are present for every preset.
This commit is contained in:
Keith Solomon
+10
-3
@@ -5,9 +5,14 @@
|
||||
* - "From" core Image: copies image attributes, sets filter="normal",
|
||||
* intensity=100.
|
||||
*
|
||||
* Per the Block Transforms API, `type: 'block'` transforms must return a
|
||||
* block instance produced by `createBlock` from `@wordpress/blocks` — a
|
||||
* raw attribute object is silently ignored and the transform fails.
|
||||
*
|
||||
* @package SoloFiltersImageEnhancements
|
||||
*/
|
||||
|
||||
import { createBlock } from '@wordpress/blocks';
|
||||
import { DEFAULT_PRESET } from './presets';
|
||||
|
||||
const IMAGE_ATTR_MAP = {
|
||||
@@ -69,16 +74,18 @@ export const transforms = {
|
||||
{
|
||||
type: 'block',
|
||||
blocks: [ 'core/image' ],
|
||||
transform: ( attributes ) => toImageAttrs( attributes ),
|
||||
transform: ( attributes ) =>
|
||||
createBlock( 'core/image', toImageAttrs( attributes ) ),
|
||||
},
|
||||
],
|
||||
from: [
|
||||
{
|
||||
type: 'block',
|
||||
blocks: [ 'core/image' ],
|
||||
transform: ( attributes ) => fromImageAttrs( attributes ),
|
||||
transform: ( attributes ) =>
|
||||
createBlock( 'ksolo/image-filter', fromImageAttrs( attributes ) ),
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
export default transforms;
|
||||
export default transforms;
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
/**
|
||||
* Manual mock for @wordpress/blocks.
|
||||
*
|
||||
* The package is provided by WordPress at runtime (via the
|
||||
* dependency-extraction-webpack-plugin) and is not installed as a
|
||||
* dev dependency. This mock lets Jest's module resolver find something
|
||||
* when the transforms test (or any other code that imports
|
||||
* @wordpress/blocks) runs in the unit-test environment.
|
||||
*
|
||||
* The mock faithfully reproduces the shape createBlock returns: a block
|
||||
* instance with `name` and `attributes` fields.
|
||||
*/
|
||||
module.exports = {
|
||||
createBlock: ( name, attributes ) => ( { name, attributes } ),
|
||||
};
|
||||
@@ -0,0 +1,63 @@
|
||||
/**
|
||||
* Regression test for the compiled filter CSS.
|
||||
*
|
||||
* Earlier the source SCSS wrote the filter rules with a descendant
|
||||
* combinator (`.wp-block-ksolo-image-filter .has-filter-warm img`), but
|
||||
* the JSX puts `wp-block-ksolo-image-filter` and `has-filter-<slug>` on
|
||||
* the SAME element, and the inner <img> carries `ksolo-image-filter-img`
|
||||
* and the same `has-filter-<slug>` class. The descendant combinator never
|
||||
* matched, so the `filter: ...` declarations were never applied — picking
|
||||
* a filter in the editor or on the front end had no visual effect.
|
||||
*
|
||||
* The fix targets the <img> by its stable, always-present pair of classes
|
||||
* (`.ksolo-image-filter-img.has-filter-<slug>`) scoped under the wrapper.
|
||||
* This test reads the compiled CSS and asserts the selectors and `filter`
|
||||
* declarations are present for every preset slug in src/presets.js.
|
||||
*
|
||||
* The test reads from the build artifact rather than the SCSS source
|
||||
* because that is the CSS WordPress actually serves. If you change
|
||||
* style.scss, re-run `npm run build` to update build/style-index.css.
|
||||
*/
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { PRESETS } from '../../src/presets';
|
||||
|
||||
const cssPath = path.join( __dirname, '..', '..', 'build', 'style-index.css' );
|
||||
|
||||
describe( 'compiled filter CSS', () => {
|
||||
let css;
|
||||
|
||||
beforeAll( () => {
|
||||
css = fs.readFileSync( cssPath, 'utf8' );
|
||||
} );
|
||||
|
||||
test.each(
|
||||
PRESETS.filter( ( p ) => p.slug !== 'normal' ).map( ( p ) => [ p.slug ] )
|
||||
)(
|
||||
'declares a filter rule for the %s preset that targets the <img>',
|
||||
( slug ) => {
|
||||
// The selector must target the <img> by its stable class pair
|
||||
// (ksolo-image-filter-img + has-filter-<slug>) scoped under the
|
||||
// wrapper, with NO space between the two class names (compound
|
||||
// selector, not descendant).
|
||||
// The 'normal' preset is unfiltered; the base
|
||||
// `.wp-block-ksolo-image-filter img { filter: none; }` rule
|
||||
// covers it, so we skip it here.
|
||||
const expectedSelector = `.wp-block-ksolo-image-filter .ksolo-image-filter-img.has-filter-${ slug }`;
|
||||
expect( css ).toContain( expectedSelector );
|
||||
}
|
||||
);
|
||||
|
||||
test( 'does not regress to the broken descendant-selector form', () => {
|
||||
// Guard against the original bug reappearing: a descendant
|
||||
// combinator between the two class names never matches the
|
||||
// actual markup, where both classes live on the same element.
|
||||
for ( const { slug } of PRESETS ) {
|
||||
if ( slug === 'normal' ) {
|
||||
continue; // 'normal' is unfiltered, no rule needed.
|
||||
}
|
||||
const broken = `.wp-block-ksolo-image-filter .has-filter-${ slug } img`;
|
||||
expect( css ).not.toContain( broken );
|
||||
}
|
||||
} );
|
||||
} );
|
||||
@@ -1,3 +1,7 @@
|
||||
// @wordpress/blocks is provided by WordPress at runtime via the
|
||||
// dependency-extraction-webpack-plugin. The Jest test environment has no
|
||||
// WordPress globals, so a manual mock lives at
|
||||
// tests/jest/__mocks__/@wordpress/blocks.js. Jest auto-resolves it.
|
||||
import { transforms } from '../../src/transforms';
|
||||
|
||||
describe( 'transforms', () => {
|
||||
@@ -25,9 +29,9 @@ describe( 'transforms', () => {
|
||||
);
|
||||
} );
|
||||
|
||||
test( 'to-transform drops filter and intensity', () => {
|
||||
test( 'to-transform produces a core/image block that drops filter and intensity', () => {
|
||||
const to = Array.isArray( transforms.to ) ? transforms.to[ 0 ] : transforms.to;
|
||||
const mapped = to.transform( {
|
||||
const result = to.transform( {
|
||||
filter: 'dramatic',
|
||||
intensity: 80,
|
||||
imageId: 12,
|
||||
@@ -38,15 +42,20 @@ describe( 'transforms', () => {
|
||||
linkUrl: '',
|
||||
caption: '',
|
||||
} );
|
||||
expect( mapped ).not.toHaveProperty( 'filter' );
|
||||
expect( mapped ).not.toHaveProperty( 'intensity' );
|
||||
expect( mapped.id ).toBe( 12 );
|
||||
expect( mapped.url ).toBe( 'https://example.com/a.jpg' );
|
||||
// WordPress block-to-block transforms must return a block instance
|
||||
// (created via createBlock), not a bare attribute object.
|
||||
expect( result ).toBeDefined();
|
||||
expect( result.name ).toBe( 'core/image' );
|
||||
expect( result.attributes ).toBeDefined();
|
||||
expect( result.attributes ).not.toHaveProperty( 'filter' );
|
||||
expect( result.attributes ).not.toHaveProperty( 'intensity' );
|
||||
expect( result.attributes.id ).toBe( 12 );
|
||||
expect( result.attributes.url ).toBe( 'https://example.com/a.jpg' );
|
||||
} );
|
||||
|
||||
test( 'from-transform normalises filter and intensity to defaults', () => {
|
||||
test( 'from-transform produces a ksolo/image-filter block with normalised filter and intensity', () => {
|
||||
const from = Array.isArray( transforms.from ) ? transforms.from[ 0 ] : transforms.from;
|
||||
const mapped = from.transform( {
|
||||
const result = from.transform( {
|
||||
id: 12,
|
||||
url: 'https://example.com/a.jpg',
|
||||
alt: 'A',
|
||||
@@ -55,12 +64,15 @@ describe( 'transforms', () => {
|
||||
height: 100,
|
||||
href: 'https://example.com',
|
||||
} );
|
||||
expect( mapped.filter ).toBe( 'normal' );
|
||||
expect( mapped.intensity ).toBe( 100 );
|
||||
expect( mapped.imageId ).toBe( 12 );
|
||||
expect( mapped.imageUrl ).toBe( 'https://example.com/a.jpg' );
|
||||
expect( mapped.imageAlt ).toBe( 'A' );
|
||||
expect( mapped.caption ).toBe( 'cap' );
|
||||
expect( mapped.linkUrl ).toBe( 'https://example.com' );
|
||||
expect( result ).toBeDefined();
|
||||
expect( result.name ).toBe( 'ksolo/image-filter' );
|
||||
expect( result.attributes ).toBeDefined();
|
||||
expect( result.attributes.filter ).toBe( 'normal' );
|
||||
expect( result.attributes.intensity ).toBe( 100 );
|
||||
expect( result.attributes.imageId ).toBe( 12 );
|
||||
expect( result.attributes.imageUrl ).toBe( 'https://example.com/a.jpg' );
|
||||
expect( result.attributes.imageAlt ).toBe( 'A' );
|
||||
expect( result.attributes.caption ).toBe( 'cap' );
|
||||
expect( result.attributes.linkUrl ).toBe( 'https://example.com' );
|
||||
} );
|
||||
} );
|
||||
} );
|
||||
|
||||
Reference in New Issue
Block a user