Skip to content
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

Raw handling: preserve image ID #6638

Merged
merged 2 commits into from
May 11, 2018
Merged

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented May 8, 2018

Description

When copy pasting an image from the old editor, or converting a classic block to blocks, image IDs are dropped. We should preserve this information so the the correct image is selected in the media library on editing the image, and so that the image is made responsive on the front-end.

How has this been tested?

Convert some old WP content to blocks.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ellatrix ellatrix added this to the 2.9 milestone May 8, 2018
@ellatrix ellatrix requested a review from a team May 8, 2018 10:59
);
Array.from( node.classList ).forEach( ( name ) => {
if ( find( classes, ( item ) => {
return typeof item === 'string' ? item === name : item.test( name );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had trouble interpreting this return as not being part of the forEach but rather an inline predicate for the find. Can we extract this out? Or maybe normalize the classes schema item to a function?

classes = classes.map( ( item ) => {
	if ( typeof item === 'string' ) {
		return ( className ) => className === item;
	} else if ( item instanceof RegExp ) {
		return ( className ) => item.test( className );
	}
} );

// ...

const newArray.from( node.classList ).forEach( ( name ) => {
	if ( ! classes.some( ( isMatch ) => isMatch( name ) ) {
		node.classList.remove( name );
	}
} );

@@ -61,7 +61,7 @@ const blockAttributes = {
const imageSchema = {
img: {
attributes: [ 'src', 'alt' ],
classes: [ 'alignleft', 'aligncenter', 'alignright', 'alignnone' ],
classes: [ 'alignleft', 'aligncenter', 'alignright', 'alignnone', /wp-image-\d+/ ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid image class: the-dog-yawp-image-1 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, sorry

@@ -101,10 +101,13 @@ export const settings = {
isMatch: ( node ) => node.nodeName === 'FIGURE' && !! node.querySelector( 'img' ),
schema,
transform: ( node ) => {
const matches = /align(left|center|right)/.exec( node.className );
const align = matches ? matches[ 1 ] : undefined;
const className = node.className + ' ' + node.querySelector( 'img' ).className;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is being done here with the concatenation?

Can you add an inline comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll add a comment. The class we need here is on the image, and for alignment it would be good to look at both the image and figure. Doesn't hurt.

@ellatrix ellatrix merged commit 0ab0deb into master May 11, 2018
@ellatrix ellatrix deleted the add/raw-handling-preserve-image-id branch May 11, 2018 17:34
@mtias mtias added the [Feature] Media Anything that impacts the experience of managing media label May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Media Anything that impacts the experience of managing media [Feature] Paste
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants