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

Visual Editor: Adding the global controls concept #443

Closed
wants to merge 4 commits into from

Conversation

youknowriad
Copy link
Contributor

In this PR. I'm exploring the idea of global controls as an alternative to #416 (image alignments)

Global controls are controls that can be automatically applied to any block type. I'm using the block positioning controls (float left, right, center) as an example.

For now these controls are automatically enabled for all blocks but I'm thinking of introducing an opt-in (or opt-out) attribute in the Block API.

Testing instructions

  • Try using the positioning controls in different blocks. (image, text, quote)

@youknowriad youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label Apr 18, 2017
@youknowriad youknowriad self-assigned this Apr 18, 2017
@youknowriad youknowriad requested a review from aduth April 18, 2017 12:54
@mtias
Copy link
Member

mtias commented Apr 18, 2017

These are inherently different for text (alignment) and media (floating props), no?

@youknowriad
Copy link
Contributor Author

youknowriad commented Apr 18, 2017

yes, our current mockups leave those for media only and I can do as well in this PR with an opt-in attribute but we discussed this somewhere with @jasmussen and It makes sense for quotes, text etc... to be floated as well.

Text can have both, alignement and floating

@mtias
Copy link
Member

mtias commented Apr 18, 2017

I'm a bit wary of adding floating options to all those elements, because it's really hard to control as a user and easily starts to behave oddly as soon as you have a few. What are we trying to solve with floating text and quotes?

@youknowriad
Copy link
Contributor Author

I'm a bit wary of adding floating options to all those elements,

I understand the concern and I'll address it by opting-in to these controls only for media blocks. What I'm exploring here is the possibility to have controls already defined, the block author doesn't have to define them in his block settings because we need them "outside" the block edit function.

If you compare these controls with the solution explored #416, you should notice that the block controls move next to the block content when floated. This is because the classname is applied in the container div instead of leaving this to the block author inside the edit function.

@jasmussen
Copy link
Contributor

This is a great exploration, thank you for working on this.

I would put a pause on it for now, though, not because it's not a good feature to have, but because we might want to put controls like this in a different place altogether.

Philosophically, the editor is designed around a type type type, then click click click formula, where the most casual user with the sidebar toggled off, needs to be able to easily write a post and then in a few clicks make a rich layout.

If you want to create a more advanced layout, one that might involve dropcaps or parallax scrolling, or things in that vein, you need to open the sidebar. The sidebar is essentially the interface separation between what is basic, and what is advanced.

When no block is selected, you see the usual metaboxes:

When you click a block, the sidebar is replaced by an Inspector, that shows advanced controls:

By putting advanced controls in the inspector only, we can keep the user interface as lightweight and contextual as possible for the block-docked controls, while still allowing advanced layouts to take place. This is likely to be even more important later in the year for the customizer.

Given that we won't be looking at columns for the initial version of the editor, it feels like text alignments are sufficient for text blocks. Though for the quote block it might be interesting to not show text alignments, but rather block alignments, so we can ideally do something like the pulled-to-the-side pullquote on this example post.

@youknowriad
Copy link
Contributor Author

youknowriad commented Apr 19, 2017

In 222c6fd I'm adding the "opt-in" mechanism. A block can decide to include automatically the global controls, we do so by adding a string corresponding to the control's slug in the blocks' controls array.

For now the global controls are just a static array but we could easily image extending those using the proposed registerControl API.

Only the image block is opting-in to have these controls.

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.

This feels a bit premature as anticipating common behaviors that may or may not be as similar as we expect. Already seeing a couple of differences:

  • We're forced to use a more generic name "Position" than "Align" or "Float"
  • In an image block, "None" is the default align, but in a text block "Left" is the default align. This is not reflected in the default control for left positioning.

const controls = settings.controls && compact(
settings.controls.map( ( control ) => {
if ( isString( control ) ) {
return wp.blocks.controls.find( ctrl => ctrl.slug === control );
Copy link
Member

@aduth aduth Apr 19, 2017

Choose a reason for hiding this comment

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

More performant to shape controls as an object keyed by slug to avoid iterating the entire set.

@youknowriad
Copy link
Contributor Author

We're forced to use a more generic name "Position" than "Align" or "Float"

Absolutely, this is a drawback. We'll have "reserved" attributes, we could decide to store these attributes in a different object than the "user"'s attributes object.

In an image block, "None" is the default align, but in a text block "Left" is the default align. This is not reflected in the default control for left positioning.

Not true because we're not talking about Text Alignment, but Block Alignment and both defaults are "None" for me.

@youknowriad
Copy link
Contributor Author

So, let's take a step back.

This PR:

  • Adds global controls concept enabling the floating controls for images
  • The alignment work great (moving the controls along the block)
  • I'm seeing the "width" as a global control too (configurable in the inspector probably) and could be implemented similar to the alignment controls here
  • The drawback of this PR is the "reserverd" position attribute (we could avoid reserving attributes if we create another object holder for global attributes)

The simpler alternative in #416

  • Adds the image alignment as regular controls
  • The drawback is that moving the controls and updating the block container height/width along the block is not possible right now

IMO, this is good enough, but I understand the "complexity" concerns. If you think, this is too premature to introduce, I'll close the PR and let's move with #416 for now.

@jasmussen jasmussen mentioned this pull request Apr 20, 2017
15 tasks
@aduth
Copy link
Member

aduth commented Apr 21, 2017

I'm still not quite comfortable with the ideas here, or would at least prefer to wait 'til their need becomes more obvious (specifically concepts of global and reserved attribute names, block rendering awareness of specific attributes).

@youknowriad
Copy link
Contributor Author

@aduth Reasonable to wait here. Let's close this for now.

@ellatrix ellatrix added this to the Prototype Parity milestone Apr 26, 2017
@ntwb ntwb deleted the try/global-controls-concept branch August 5, 2017 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants