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

Can't access className from a block's save function #7308

Closed
joemcgill opened this Issue Jun 14, 2018 · 23 comments

Comments

Projects
None yet
7 participants
@joemcgill
Copy link
Contributor

joemcgill commented Jun 14, 2018

Describe the bug
At times, it's useful to reference the className property of a block in the save function. This is currently not possible, as the className value is not passed to the save function for a block, even though several core blocks expect to receive a className argument (e.g., cover image, subhead, verse).

For example, if you want to use the default className value in your save function to dynamically make your block use a BEM like class syntax, you might write something like this:

save( { attributes, className } ) {
	const { content, title } = attributes;

	return (
		<div>
			<h3 className={ `${ className }__title` }>{ title }</h3>
			<p className={ `${ className }__content` }>{ content }</p>
		</div>
	)
}

However, className is undefined, so this doesn't work, even though several core blocks (as noted) show this exact argument signature. In those cases, the undefined className value is hidden by the fact that it is passed, along with other class names to the classnames() function or as the className attribute of a RichText.Content component, both of which hide the fact that this value is undefined.

To Reproduce
Steps to reproduce the behavior:

  1. Set a breakpoint inside the save function of the cover image block;
  2. Inspect the value of the className argument and see that it's undefined.

Expected behavior
The className value should be passed as an argument to the save function.

@danielbachhuber

This comment has been minimized.

Copy link
Member

danielbachhuber commented Jun 14, 2018

@joemcgill Can you update the description to include a representative snippet of code?

even though several core blocks expect to receive a className argument (e.g., cover image, subhead, verse).

What happens in this scenario?

@joemcgill

This comment has been minimized.

Copy link
Contributor Author

joemcgill commented Jun 15, 2018

@danielbachhuber I've updated the description with a reduced case and explain how the few core blocks that include a className attribute are hiding the fact that this value is undefined.

In my code example above, I don't know if there is a workaround that wouldn't require explicitly passing the className value to the save function of the block or if this should simply not be supported by the blocks API, but if the latter, the core blocks that are currently expecting className should be updated to remove that value.

@danielbachhuber

This comment has been minimized.

Copy link
Member

danielbachhuber commented Jun 15, 2018

Thanks @joemcgill

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Jun 15, 2018

Not sure this directly solves it, but if you add className as an attribute on your block, it gets passed around as an attribute.

(This can lead to another issue where blocks that don't explicitly support className as an attribute can break attribute validation when a class is added to the Additional CSS Classes field, which is always visible. I don't have a test case for this but can submit an issue once I do.)

@joemcgill

This comment has been minimized.

Copy link
Contributor Author

joemcgill commented Jun 15, 2018

Thanks for the suggestion @chrisvanpatten. I guess specifically duplicating the className as an attribute is an option, but seems duplicative. A nice workaround for the moment though.

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Jun 15, 2018

@joemcgill looking at the Cover Image block, it looks like the className is being passed in as attributes.className. Not sure how it's actually getting serialised since obviously the top-level className prop is undefined, and attributes.className isn't referenced at all in the save() callback, but there you go. Some weird magic under the hood…?

@gziolo

This comment has been minimized.

Copy link
Member

gziolo commented Jul 10, 2018

@joemcgill looking at the Cover Image block, it looks like the className is being passed in as attributes.className. Not sure how it's actually getting serialised since obviously the top-level className prop is undefined, and attributes.className isn't referenced at all in the save() callback, but there you go. Some weird magic under the hood…?

All correct @chrisvanpatten. This is how you can reference className in the save function. By default is always injected to the root element. If you want to use it elsewhere, this is a way to go.

@ocean90

This comment has been minimized.

Copy link
Member

ocean90 commented Nov 7, 2018

@gziolo Can you elaborate on this? Should this work or not? When using save( { attributes, className } ) { } then className is always undefined. I tested this in a custom block but also with the cover/subhead/verse blocks. It's also mentioned in the attributes docs.

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 7, 2018

@ocean90 I think the way it works now is

save( { attributes } ) {
  console.log(attributes.className);
}

So className isn't a top-level prop, but located under attributes. (I could be wrong on this, but that's what I've been doing!)

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 7, 2018

(So potentially, this should get a Documentation flag.)

@ocean90

This comment has been minimized.

Copy link
Member

ocean90 commented Nov 7, 2018

@chrisvanpatten Unfortunately there's also no attributes.className.

@gziolo

This comment has been minimized.

Copy link
Member

gziolo commented Nov 7, 2018

I think that className is something you can fill in in the sidebar in Advanced section. Once you provide it, it should be exposed under attributes.className. In the case of generated class name, it's applied after save method is called:
https://github.com/WordPress/gutenberg/blob/master/packages/blocks/src/api/serializer.js#L74

using filter:
https://github.com/WordPress/gutenberg/blob/master/packages/blocks/src/api/serializer.js#L86

So you can't really access it. The idea is that the generated class name is always applied to the root element.

@ocean90

This comment has been minimized.

Copy link
Member

ocean90 commented Nov 7, 2018

Related: #2035 (cc @aduth)

@ocean90

This comment has been minimized.

Copy link
Member

ocean90 commented Nov 7, 2018

@gziolo Thanks for your answer! So the linked references should probably be updated?

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 7, 2018

The generated className isn't auto-added in the save handler anymore, and certainly wouldn't work for server rendered blocks.

I think there are a few potential paths:

  1. Always pass the generated classname through attributes
  2. Leave the behaviour the same, and improve the documentation to explain what's going on

I think either way we should audit the core blocks to be sure they aren't pulling in undefined className props as it sounds like they may still be using this legacy method.

Sounds like option 2, plus an audit of the blocks that are _doing_it_wrong is the way forward :)

@aduth

This comment has been minimized.

Copy link
Member

aduth commented Nov 8, 2018

The documentation has since been updated in #11605. Should this then be closed?

Otherwise, what are the specific action items of said audit? (What measure can be made for closure of the issue?)

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 8, 2018

@aduth merging #11605 will close this.

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 8, 2018

Oops. This can be closed! Just saw it was merged.

@mrmadhat

This comment has been minimized.

Copy link
Contributor

mrmadhat commented Nov 19, 2018

I know this was closed because classNames was removed from the save methods of core blocks but what do you do if you want access to the classes in the save method to allow BEM like class syntax without manually having to add a class in the editor?

For example, if you want to use the default className value in your save function to dynamically make your block use a BEM like class syntax, you might write something like this:

save( { attributes, className } ) {
	const { content, title } = attributes;

	return (
		<div>
			<h3 className={ `${ className }__title` }>{ title }</h3>
			<p className={ `${ className }__content` }>{ content }</p>
		</div>
	)
}
@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 19, 2018

@mrmadhat You can just specify className in your block code, e.g. const className = 'my-class-name';

@ocean90

This comment has been minimized.

Copy link
Member

ocean90 commented Nov 19, 2018

Or use const className = getBlockDefaultClassName( 'your/block-name' ). It's part of @wordpress/blocks.

@chrisvanpatten

This comment has been minimized.

Copy link
Member

chrisvanpatten commented Nov 19, 2018

@ocean90 TIL… that's super handy!

@mrmadhat

This comment has been minimized.

Copy link
Contributor

mrmadhat commented Nov 19, 2018

Thanks @ocean90 that's perfect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.