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

Gutenberg Block for Tweet Shortcode? #7597

Closed
wants to merge 21 commits into from

Conversation

georgestephanis
Copy link
Member

Work In Progress!

@georgestephanis georgestephanis requested a review from a team as a code owner August 4, 2017 21:41
@georgestephanis georgestephanis removed the request for review from a team August 4, 2017 21:41
@georgestephanis georgestephanis self-assigned this Aug 4, 2017
category: 'layout',

attributes : {
tweet : wp.blocks.query.query( 'input[name=tweet]').value
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea with attribute matchers (or, perhaps soon to be called "sourced" values) is to avoid duplicating data between comment attributes and the actual saved markup of the block. For example, we don't need to serialize an image's URL into the comments blob if we know we can easily grab it from the img's src attribute. In more complex examples, duplication is inevitable or there's a need for hidden metadata.

So what we define in attributes purely relates to getting an object representation of the block back from the saved content, nothing to do with the edit-able render.

In your case, the saved content is a shortcode, from which you want to extract the tweet value. There's not a matcher for this (arguments could be made either way for whether we should add conveniences for shortcodes), so you might just want to let this live in comment metadata. Currently this is automatic if you set an attribute from your edit form but don't define it in the attributes property, but this is likely to change soon to be more explicit (i.e. define all attributes, both the ones sourced from content and those serialized into comments).

I'm supposing you were expecting that defining this here would establish a link between the attribute value and the input from edit, which is not the case. Instead, you'll want to explicitly call props.setAttributes from your edit function when the input changes, similar to the concept of a controlled input in React.

onInput: function( event ) {
	props.setAttributes({
		tweet: event.target.value
	});
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the explanation. I wasn't expecting anything really, just playing around and seeing how things connect.

Regarding extracting stuff out -- can the attribute matcher / sourced values be a custom function passed in that will return what we care about from the contents somehow? If so, I could just call wp.shortcode.next() on the contents of the block to extract and parse the shortcode and its parameters -- I've already enqueued shortcodes.js on the page for later usage building it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently this is possible by defining attributes as a function, which is passed the raw content (string) of the block and expected to return an object of attributes.

https://github.com/WordPress/gutenberg/blob/33542dd/blocks/api/parser.js#L28-L29
https://github.com/WordPress/gutenberg/blob/master/blocks/README.md#wpblocksregisterblocktype-name-string-typedefinition-object-

However, with increasing pressure to introduce server-capable attributes schemas / parsing, I'm not sure whether this can be relied upon.

Some more discussion at: WordPress/gutenberg#1905

{
name : 'tweet',
type : 'url',
value : props.tweet
Copy link
Contributor

Choose a reason for hiding this comment

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

props isn't the attributes of the block, but attributes can be accessed from props.attributes. There are a number of other values in props as well, such as setAttributes noted in the previous comment.

Instead, this would be: props.attributes.tweet

attrs : {
named : {},
numeric : [
props.tweet
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, this would be: props.attributes.tweet

};

wp.blocks.registerBlockType( 'jetpack/tweet', {
title: wp.i18n.__( 'Tweet', 'jetpack' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

A text domain solution hasn't yet been implemented for __, so the second parameter has no effect here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Created an issue at nylen/gutenberg-examples#9

@georgestephanis
Copy link
Member Author

Also, I'm not sure why, but my wp:jetpack/tweet blocks keep changing to wp:core/freeform on reloading the editor.

@georgestephanis
Copy link
Member Author

I think ^^ may be happening because I'm registering my block too late perhaps, so it's getting reset to core/freeform?

This lets it be inline on the page, with the other scripts -- rather
than printed at the very bottom.

Leaving the `script` tags there but commented out so PHPStorm still
syntax highlights it as js.
@georgestephanis
Copy link
Member Author

6817940 resolves the timing issue -- we're not printing it out manually any more, so it just goes at the right time via the core script system.

@georgestephanis
Copy link
Member Author

Somewhat interestingly, to get the dummy script to be enqueued, it has to be registered first and then seperately enqueued -- it can't just be enqueued as one step when the $src is falsey.

@aduth
Copy link
Contributor

aduth commented Aug 7, 2017

It might be good to sort through issues of adding a block registration inline, but is there a particular reason you've chosen not to create a separate .js file for your block and enqueue that?

@georgestephanis
Copy link
Member Author

I'm also getting this error:

Warning: A component is changing an uncontrolled input of type url to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://fb.me/react-controlled-components

@aduth
Copy link
Contributor

aduth commented Aug 17, 2017

This is because, since your attributes aren't being parsed, the default value for what you're assigning into the input is undefined, but then as the user types into the input is assigned a value. React treats an undefined value as an uncontrolled input and an non-empty value as a controlled input, and warns when you switch between the two. The workaround is to ensure that you never assign undefined as the value: either by making sure the attribute has a default, or providing a fallback value in the value: attributes.foo || ''

@dereksmart dereksmart added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Feb 23, 2018
@stale
Copy link

stale bot commented Jun 29, 2018

This PR has been marked as stale. This happened because:

  • It has been inactive in the past 3 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with master, and it is still valid. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label Jun 29, 2018
@dereksmart
Copy link
Member

Closing, as stale and will be picked up by another team or by George when he returns

@dereksmart dereksmart closed this Jul 11, 2018
@kraftbj kraftbj deleted the gutenberg/tweet-block branch May 24, 2019 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Shortcodes / Embeds [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files [Type] Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants