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

Add more data about the image as block props in the image block #11973

Closed
wants to merge 45 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
4eb7ee5
Add fileWidth, fileHeight, userSet, editWidth props
azaozz Nov 16, 2018
fe99150
Merge branch 'master' into fix/image-block-add-props
azaozz Nov 16, 2018
1105451
Fix upscaling
azaozz Nov 16, 2018
1c5b8a0
Fix param/prop names
azaozz Nov 16, 2018
dc8133c
Merge branch 'master' into fix/image-block-add-props
azaozz Nov 17, 2018
03444d7
Better filter name, allow valueless attributes
azaozz Nov 17, 2018
fc7c8a6
Merge branch 'master' into fix/image-block-add-props
aduth Nov 20, 2018
1aa5b35
Compat: Resolve PHP lint errors
aduth Nov 20, 2018
2a9fba9
Compat: Correct closing img tag
aduth Nov 20, 2018
04706dd
Compat: Allow optional space after img tag name
aduth Nov 20, 2018
08473d8
Compat: PHPDoc formatting and arrangement style
aduth Nov 20, 2018
32d5c8c
Blocks: Use Math.round for image rounding
aduth Nov 20, 2018
cf50aec
Blocks: Add deprecation for attribute-sourced URL, alt
aduth Nov 20, 2018
e4cfea3
Blocks: Use hard-coded content width for block width
aduth Nov 20, 2018
4c49a8d
Blocks: Limit editWith assignment to insert, resize
aduth Nov 20, 2018
f9da97f
Blocks: Rename Image userSet to userSetDimensions
aduth Nov 20, 2018
8cef6ac
Blocks: Remove redundant userSetDimensions normalization
aduth Nov 20, 2018
84d6866
Blocks: Document constrainImageDimensions as ported from PHP
aduth Nov 20, 2018
db48c26
Blocks: Avoid mutative, unused Array#map result
aduth Nov 20, 2018
b2bc0e7
Block: Avoid overloading Image block updateImageURL
aduth Nov 20, 2018
f69ec35
Blocks: Fix Image accidental inline tab character
aduth Nov 20, 2018
009eceb
Compat: Avoid assumption of ID as non-last attribute of image
aduth Nov 20, 2018
bf9d58c
Compat: Document image block RegEx as temporary solution
aduth Nov 20, 2018
51e6c96
Blocks: Pick url, alt in raw transform from image node
aduth Nov 20, 2018
174bee0
Framework: Regenerate fixtures
aduth Nov 20, 2018
c26abe2
Blocks: Update image fixtures per url, alt as comment attributes
aduth Nov 20, 2018
579cf12
Compat: Align PHPDoc return
aduth Nov 20, 2018
9563082
Compat: Add PHPDoc since tag
aduth Nov 20, 2018
6efdb5c
Merge branch 'master' into fix/image-block-add-props
aduth Nov 20, 2018
a0a6dd9
Blocks: Remove outdated comment about floated images width accuracy
aduth Nov 20, 2018
8e73feb
Compat: Remove any attributes containing invalid characters
aduth Nov 20, 2018
8535d73
Blocks: Reintegrate media attributes pick to setAttributes spread
aduth Nov 20, 2018
4d2d959
Blocks: Set Image fileWidth, fileHeight only if both present
aduth Nov 20, 2018
19dad07
Compat: Use spec standard for valid attribute characters
aduth Nov 20, 2018
87130e4
Editor: Compute expected block width by DOM compute
aduth Nov 20, 2018
e2f8007
Compat: Always write img attribute value, even empty
aduth Nov 20, 2018
bf67408
Blocks: Avoid multiple calls to setAttributes in resetWidthHeight
aduth Nov 20, 2018
d8d7d6e
Update the 25-50-75-100% buttons
azaozz Nov 23, 2018
e3a99c7
Check only for valid HTML 5.0 attribute names
azaozz Nov 24, 2018
44c7c68
Update handling of "wide" and "full" alignment for the front-end
azaozz Nov 24, 2018
652ac92
Merge master
azaozz Nov 24, 2018
078a75b
Update `const getBlockWidth` to get the width from `div.editor-block-…
azaozz Nov 24, 2018
4be9bee
Fix typo in https://github.com/WordPress/gutenberg/pull/11973/commits…
azaozz Nov 25, 2018
4b0425f
Fix missing space to make wpcs happy
azaozz Nov 25, 2018
648811f
Update `gutenberg_render_block_core_image()`
azaozz Nov 28, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Blocks: Limit editWith assignment to insert, resize
  • Loading branch information
aduth committed Nov 20, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 4c49a8dba1f5a87a467764822d7c2a356cbe975d
19 changes: 3 additions & 16 deletions packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
@@ -124,8 +124,6 @@ class ImageEdit extends Component {
const { attributes, setAttributes } = this.props;
const { id, url = '' } = attributes;

this.setEditWidth();

if ( isTemporaryImage( id, url ) ) {
const file = getBlobByURL( url );

@@ -146,8 +144,6 @@ class ImageEdit extends Component {
const { id, url = '', fileWidth } = this.props.attributes;
const imageData = this.props.image;

this.setEditWidth();

if ( isTemporaryImage( prevID, prevURL ) && ! isTemporaryImage( id, url ) ) {
revokeBlobURL( url );
}
@@ -223,6 +219,7 @@ class ImageEdit extends Component {
// Not used in the editor, passed to the front-end in block attributes.
fileWidth,
fileHeight,
editWidth: this.props.contentWidth,
} );
}

@@ -393,6 +390,7 @@ class ImageEdit extends Component {
fileWidth,
fileHeight,
userSet,
editWidth: this.props.contentwidth,
} );
}

@@ -408,6 +406,7 @@ class ImageEdit extends Component {
width: undefined,
height: undefined,
userSet: undefined,
editWidth: this.props.contentwidth,
} );
Copy link
Member

@dmsnell dmsnell Nov 19, 2018

Choose a reason for hiding this comment

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

there's no need here for two calls to setAttributes() - we can collapse them into one in a couple of different ways. my favorite is with Object.assign()

// what does this name mean?
resetWidthHeight( fileWidth, fileHeight ) {
	this.props.setAttributes( Object.assign(
		{
			width: undefined,
			height: undefined,
			userSet: undefined,
		},
		fileWidth && fileHeight && {
			fileWidth,
			fileHeight,
		}
	) );
}

Copy link
Contributor Author

@azaozz azaozz Nov 20, 2018

Choose a reason for hiding this comment

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

Sure. The purpose here is to make it as easy to read and understand as possible. After going through babel and minification, the code that actually runs in production will be completely different.

(I actually "have a beef" with the way a lot of the code is written. All the redundant indentation, nested lambda functions (no names = poor context), "clever" conditionals, almost no inline comments... There seem to be quite a few cases where the code becomes more readable after running it through babel and minification, then "unminifying" it. But lets look at that after 5.0 is out.) :)

Copy link
Member

Choose a reason for hiding this comment

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

there's no need here for two calls to setAttributes() - we can collapse them into one in a couple of different ways. my favorite is with Object.assign()

(Noting that the supplied example doesn't use Object.assign and wouldn't work as written)

Copy link
Member

@dmsnell dmsnell Nov 20, 2018

Choose a reason for hiding this comment

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

(Noting that the supplied example doesn't use Object.assign and wouldn't work as written)

Good call! 😆 (fixed)

Copy link
Member

Choose a reason for hiding this comment

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

Sure. The purpose here is to make it as easy to read and understand as possible. After going through babel and minification, the code that actually runs in production will be completely different.

The main thing for me here would be to avoid multiple calls to setAttributes, as it will incur a render for each (unnecessary impact on performance). This will remain true after transpilation and minification.

I pushed bf67408 which I think is a reasonable compromise to preserve a similar readability.

}

@@ -449,18 +448,6 @@ class ImageEdit extends Component {
};
}

/**
* Update the block's `editWidth` attribute if not aligned to the current
* `contentWidth` prop.
*/
setEditWidth() {
const { attributes, setAttributes, contentWidth } = this.props;
const { editWidth } = attributes;
if ( contentWidth !== editWidth ) {
setAttributes( { editWidth: contentWidth } );
}
}

getFilename( url ) {
const path = getPath( url );
if ( path ) {