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

A block API proposal v2. :) #69

Closed
wants to merge 1 commit into from
Closed

A block API proposal v2. :) #69

wants to merge 1 commit into from

Conversation

BE-Webdesign
Copy link
Contributor

@BE-Webdesign BE-Webdesign commented Feb 14, 2017

Hi!

Here is my second attempt at an API for creating blocks. To learn more about the API design check out https://github.com/BE-Webdesign/gutenberg/blob/block-api-2/src/api/index.md. Following the theme of "block" first, the basic gist of the API focuses on creating a "block" that can be used to create actual implementations of blocks and controls. These blocks and controls can be composed of themselves to make bigger, different blocks and controls.

This currently only implements the block controls on the text elements, images still are running off of what is currently being used.

Would love to bring discussion about this idea. Explore, test, and try it out. Let me know your thoughts!

@dmsnell
Copy link
Member

dmsnell commented Feb 14, 2017

thanks @BE-Webdesign! so much code 😄 and well done on the descriptive README 👏

there are points in this PR I like and there are things I get lost in, such as the concrete realization in code of all these concepts. the points about composability and extendability of blocks is a good thing. I would like to know more how you envision seeing this from a user's perspective. We have discussed, for example, the idea of graduating a text block into a map block if it contains an address. How would you see this happening with your proposed API? How about going in the reverse direction?

questioning the complexity is another point to pause on. in the example code, even though it's demonstrating an extreme, I think that the use of textBlock()()()()()()… is definitely the kind of thing which could lead off to a rabbit-chase in design.

anyway, these aren't any major or official comments, just my own first response.

@BE-Webdesign
Copy link
Contributor Author

Hi @dmsnell ,

well done on the descriptive README

More to come hopefully!

there are things I get lost in, such as the concrete realization in code of all these concepts

Yeah, the concrete part is not the best representation of these ideas, due to lack of time. I can definitely clean up that code and expand on things to give a clearer picture (hopefully). This is mostly an experiment with a concept that I think could work well; maybe it won't. Also feel free to add line notes for where things are becoming too obscure.

I would like to know more how you envision seeing this from a user's perspective.

The user in this case being the developer?

We have discussed, for example, the idea of graduating a text block into a map block if it contains an address. How would you see this happening with your proposed API?

Do you mean there is already a text block present and the user entered something like "1 Cupertino Infinite Loop" as it's value and then it gets turned into a map block of that address? If I understand correctly, that's a cool idea. I think it wouldn't really be necessary though. If a user wants to add a special block, like a map, I imagine giving them a button, palette of buttons, and/or keyboard shortcut to create new blocks would be easier from a development perspective as well as a user perspective. They could enter the address after they create the map block. I think that model would be more intuitive and fit user intent. The "typing /map" idea is good too, for creating new blocks. In any case all of these possibilities would just boil down to event handlers which would be attached onto a block, or a control.

How about going in the reverse direction?

The transforming of blocks will definitely be tricky, I would love to hear your ideas on how you think it could be solved. An initial thought would be to have every block have a method that will transform it into a "base" block type. Then all other block types would have a way to read this "base" block type and interpret the data input the best they can.

Start with a paragraph containing "1 Cupertion Infinite Loop": user triggers some action to change the block into a open street map block, the properties of the paragraph are condensed into a very basic block with a set interface. The text in the paragraph could become a basic data property on this "base block", then the base block would then become the data input for the open street map block to interpret and render. If the paragraph only contained "I love gardening!", then the map block as it renders could inform the user that it could not locate "I love gardening!". The base block would be handled internally and the user would not experience that change.

Paragraph -> user wants to change to map -> base block -> interpreted by map block.

text -> base -> map then convert back map -> base -> text

I think that the use of textBlock()()()()()()… is definitely the kind of thing which could lead off to a rabbit-chase in design.

Definitely, a concern of mine. However, I honestly think the approach this PR is heading in is less error prone than other ways of working in JavaScript; like relying on this. The intent of this PR is to provide a way to rapidly create blocks and extend the editor, as well as provide a base to pivot from as things change. However, if ultimately things are confusing, then ¯_(ツ)_/¯ it was not useful.

anyway, these aren't any major or official comments, just my own first response.

I greatly appreciate the feedback and getting the dialog started! I should be able to get more of this fleshed out over the coming weeks, to hopefully provide a clearer picture and demonstrate ease of extending the editor. At this point, things are probably too abstract for me to even know whether this will work 😄 .

@sirbrillig
Copy link

I'm still grokking this in my head, but initially I really like its functional and composeable nature. In my experience JavaScript devs get into the most trouble when using this, prototype, and new (and now class), and I love that this avoids all of them!

@nb
Copy link
Member

nb commented Feb 15, 2017

Do we have a list of use-cases for the API, so that we can see how would each proposal cover them?

@nylen
Copy link
Member

nylen commented Feb 16, 2017

I'm still grokking this in my head, but initially I really like its functional and composeable nature.

Same. Also, I really really like that the majority of the code here is test cases - we need to make sure all of our work on this project has at least this level of test coverage.

@BE-Webdesign
Copy link
Contributor Author

Do we have a list of use-cases for the API, so that we can see how would each proposal cover them?

I can draft something up that will cover the advantages/how to use this approach. Going to do a full implementation soonish using this approach, so it should hopefully be clearer how it works/how to use it.

@nb
Copy link
Member

nb commented Feb 16, 2017

@BE-Webdesign thanks, knowing more about advantages/disadvantages would be great.

The question, however, was about use-cases or flows, for which the API would be used. Just like when designing a user interface we are looking at all the flows and use-cases of the system, when designing an API, it would be super useful to have a list of what a developer should be able to achieve with it.

@BE-Webdesign
Copy link
Contributor Author

The question, however, was about use-cases or flows... it would be super useful to have a list of what a developer should be able to achieve with it.

Thank you for clarifying. I will have to think about the different use cases a developer might run into and show how to use this for that. Going to accompany it with more working code too.

@BE-Webdesign
Copy link
Contributor Author

Is it okay to introduce a build step and use ES6 with Babel? I wasn't 100% sure what type of JS should be written for WordPress core?

@mtias
Copy link
Member

mtias commented Feb 17, 2017

@BE-Webdesign this prototype would allow you to do that. It'd also be fine to do a separate one (add a folder called API) just exploring an API if you prefer, but the UI one should remain just about UI interactions and hopefully just vanilla JS for now.

@BE-Webdesign
Copy link
Contributor Author

Okay, thank you mtias. When I have time to work on this, I will make sure to put everything into a separate folder, and keep the UI part vanilla JS. Hope to have something more concrete soon.

@aduth
Copy link
Member

aduth commented Feb 17, 2017

Related:

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Great docs and reasoning (love seeing both pros/cons outlined). Left a few comments to try to clarify some of the reasoning. Admittedly I think I'm biased more toward using simple JavaScript objects than introducing specific creator APIs, even if just to reduce the barrier to entry.

Per some of the earlier comments, I think we ought to start a discussion (issue) to outline the general requirements of an API (#27 was apparently meant to be targeted at the UI prototype).

type: 'wp-text',
title: 'Text',
controls: [
[
Copy link
Member

Choose a reason for hiding this comment

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

Based on usage in later files, not sure you'd meant for this to be a nested array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah :(, the docs are probably confusing. That looks like an error to me 😄.

After I learn more about Peg.js and TinyMCE, I should have a working prototype accompanying this PR with better documentation.

title: 'Text',
controls: [
[
controlType( {
Copy link
Member

Choose a reason for hiding this comment

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

What advantage is there to defining controls using the controlType wrapper function? In your previous pull request reflections you'd mentioned how this might help with composing controls, though I'm curious what the practical applications would be, and whether it's a common and painful enough problem that warrants additional APIs.

The alternative I see is simply passing each control as an object. And as far as composition goes, you could still extend one control with another through e.g. Object.assign.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think of the various Types as a factory or way to create objects of itself. {block/control}Type serve more as an object describing what an actual block or control should look like. When you need an actual text block you would just call: textBlockType.create( state ), if you need another text block call that again. Then the blocks would exist somewhere and if you need to render their state you would call render() on the blocks that you needed to render.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alternative I see is simply passing each control as an object. And as far as composition goes, you could still extend one control with another through e.g. Object.assign.

Essentially all this is a shorthand for what you are describing. When ever you call control or controlType or whatever you are just creating a new copy of that function with the new properties assigned via Object.assign().

controlType( {
type: 'left-align',
title: 'Align Left',
displayArea: 'block'
Copy link
Member

Choose a reason for hiding this comment

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

Had you considered making displayArea a key of controls or its own top-level key? e.g.

controls: {
	block: [ ... ],
	inline: [ ... ]
}
blockControls: [ ... ],
inlineControls: [ ... ]

Do you see an advantage to making it a property of the control itself?

Copy link
Contributor Author

@BE-Webdesign BE-Webdesign Feb 20, 2017

Choose a reason for hiding this comment

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

Do you see an advantage to making it a property of the control itself?

Yes.

This ties in with the above, comment. If what I am saying doesn't make sense I can Google hangout or something and probably explain it better.

Think of everything as data. It can be transformed and turned into any format you want. If you have these as properties on the control object itself, then when iterating/looping (whatever you want to call it ) you can create new lists of controls more easily. If you prematurely group the controls as an object to get all of the controls to iterate over you would need to iterate over Object.keys() first and then match that against the object to get the actual value of the control. This gets really cumbersome when you are doing deep iteration and when you want to iterate across all of the controls on a different grouping. Keeping controls as an array then using map() or forEach() or a regular old for loop, for me makes it easier to work with them; just a style choice I guess. But typically if you keep them as an array of objects it will make them more flexible, and lead to less code writing.

controls: [
[
controlType( {
type: 'left-align',
Copy link
Member

Choose a reason for hiding this comment

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

What purpose is type serving? Is this a unique identifier for the control? If so, is it unique across all blocks (allowing controls to be reused)? Could this be a key on the controls object?

{
	controls: {
		'left-align': { ... }
	}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What purpose is type serving? Is this a unique identifier for the control? If so, is it unique across all blocks (allowing controls to be reused)?

Yes

Could this be a key on the controls object?

You can do that, but I would not suggest it. If you have a list of controls it should be a list of control objects. Not a key value storage. Obviously that is preference, but overtime I have drifted to the idea that if it is a list of something it should indeed be an array. This makes iterating over it a lot easier. If we wanted to iterate over the types of controls by changing it to an object we would need to iterate over object keys and match that to the object. Instead of just looping through the controls.

const textBlockType = blockType( textBlockTypeProperties )

// Create a text block.
const aTextBlock = textBlockType.create()
Copy link
Member

Choose a reason for hiding this comment

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

I wasn't totally clear with your previous remarks the intent behind having both create and separately a render function. Could we achieve the same by just creating plain objects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essentially all this API does is create plain objects. It becomes a shorthand for creating objects so you write less code. Technically the objects are functions, but in JS, they are the same thing. The documentation is not complete, because the implementation is not complete. Think of the types as a way to create objects of that type.

// containerProperties's render() method could provide a way to render both aTextBlock and anotherTextBlock inside of it.
const blockContainer = block( containerProperties )

let container = containerType(
Copy link
Member

Choose a reason for hiding this comment

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

Could you clarify and/or add docs around what role a container is meant to serve?

Copy link
Contributor Author

@BE-Webdesign BE-Webdesign Feb 20, 2017

Choose a reason for hiding this comment

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

Yup :). I imagine at some point nested blocks will be something people want, i.e. multi column layouts, galleries. A container would serve as a block that just renders blocks contained inside of it. Its controls would be to add more blocks or remove blocks. Since there is an aversion to doing nested blocks upfront, it is good to at least map out how that would work with an API. I wouldn't worry about this too much, just an idea.

render: ( state = {}, props = {} ) => {
// Do some stuff relating to the blockType for this block.

// Render something. Could be DOM, vDOM, React, TinyMCE, JSON, XML, HTML, whatever you want.
Copy link
Member

Choose a reason for hiding this comment

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

Would we expect developers to implement all/each of these, or is this just outlining all of the options we would want to support? Should we have a preference toward one single recommendation for rendering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would we expect developers to implement all/each of these, or is this just outlining all of the options we would want to support? Should we have a preference toward one single recommendation for rendering?

This could be whatever you want it to be, but probably it should be a set interface like this taking state and props. Potentially it should just be a state object. I did state props here to match React.

@BE-Webdesign
Copy link
Contributor Author

BE-Webdesign commented Feb 20, 2017

Admittedly I think I'm biased more toward using simple JavaScript objects than introducing specific creator APIs, even if just to reduce the barrier to entry.

Yeah, I am not too stuck on anything and mainly want to help out. So whatever API is picked, I will help out with that. I can understand that looking at the low level side of things is maybe confusing, but to me something like:

var quoteType = blockType( textType, quoteProperties )

is easier to understand than:

var quoteType = Object.assign( {}, textType, quoteProperties )

So the JavaScript weirdness (which really isn't that weird, I will try to rename things so it is more easily understood, and better document) happening in blockType et al. is somewhat justified. If you understand Object.assign(), you can definitely understand how this API works.

Per some of the earlier comments, I think we ought to start a discussion (issue) to outline the general requirements of an API (#27 was apparently meant to be targeted at the UI prototype).

Definitely, I think we want to define, what we need now, and also potentials in the future, to make sure the API that is created does not tie us down to our current perception of creating the editor. Future stuff is way less important, but it shouldn't be made difficult to implement by not at least thinking about it.

@BE-Webdesign
Copy link
Contributor Author

BE-Webdesign commented Feb 20, 2017

@aduth Created #104, would love to hear your thoughts in there, as well as everyone elses. Also great job on your prototype. Very cool. The Peg.js stuff is over my head haha, trying to learn it right now, it is pretty awesome.

@BE-Webdesign
Copy link
Contributor Author

Closing, as I think the block type definition API should probably be in PHP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants