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

Attributes: Add support for site options #3112

Closed
wants to merge 2 commits into from

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented Oct 23, 2017

Implements #2759

The essence

This pull request aims to add support for site options as an attribute source:

attributes: {
  siteDescription: {
    type: 'text',
    option: 'description'
  }
},

edit( { attributes, setAttributes, focus } ) {
  return [
    focus && <InspectorControls  />,
    <input
      value={ attributes.siteDescription }
      onChange={
        ( event ) => setAttributes( {
          siteDescription: event.target.value,
        } )
      } />,
  ];
}

gutenberg-site-options

The first commit corresponds to the initial exploration, which works, though for simplicity it resorted to the pre-Gutenberg Backbone WP-API interface—wp.api.models.Settings—with an instance living in editor/effects to handle reads (populating Redux state via action dispatching) and writes (Model#save method).

Next: withAPIData

Following that exploration, thought was given to putting withAPIData (WAD) to use, in order to interact with WP-API at /wp/v2/settings. Why? To see how far WAD can help Gutenberg satisfy its data needs across the board, to see if we can identify bits of it that could be improved.

One-off, isolated use of site options

However, WAD is a higher-order component, thus designed to enhance components by injecting its data (which incidentally gets its own cache, invisibly shared across WAD-enhanced instances). It arguably has more local data needs in mind (for a specific component needing a specific external resource), potentially at odds with a new kind of attribute that is global to a site.

WAD’s ideal scenario it to enhance a block directly and statically (cf. example). However, we don’t want this explicit enhancement to be a burden for block authors seeking to interact with site options. Suppose, then, an enhancer—for now named connectSiteOptions—that builds on WAD:

const connectSiteOptions = ( typeAttributes ) => flowRight(
  withAPIData( constant( { settings: '/wp/v2/settings' } ) ),
  withInterceptedAttributes( typeAttributes )
);

Block = connectSiteOptions( typeAttributes )( Block );

This is straightforward to implement. withInterceptedAttributes would inject the requested site options into attributes, and wrap setAttributes so as to intercept and handle attempts to set site options. Nonetheless, it shows limitations as soon as more than one block needs site options. For instance, if a block allows the user to change a site's title, should other blocks that present the site title immediately reflect the local change? Should they only update once that change is pushed to the server?

Stretching the mechanism too far?

It would possible to make this scale for multiple blocks the following way:

  • Some top-level component like VisualEditorBlockList would be enhanced with a HoC that would keep its own state on site options.
  • Either through React context injection or through prop passing, it would expose bound methods (e.g. getSiteOptions, setSiteOptions) to its descendants.
  • BlockEdit instances would be enhanced with a HoC that would intercept attributes and their setter, just as before, but relaying this data to and from the ancestor (pseudo-code: if attribute.name in siteOptionAttributes; then setRealAttribute(attribute); else setSiteOption(attribute).

I got this to work conceptually, but it doesn't seem like anything we'd want.

As a virtual data-requesting component

Instead of working against Gutenberg's architecture, and namely its approach of centralized state, we could have something similar to Calypso's query components sitting in Layout or VisualEditor:

class SiteOptionsManager extends Component {}
export default flowRight(
  withAPIData,
  connect(  ) // to detect post saves & to dispatch actions
)( SiteOptionsManager );

This still feels, in some way, like a suboptimal solution when compared to the existing paradigm in Gutenberg: views can dispatch actions signalling intent (to have data, to set/save it), effects react to these actions by interacting with the network and dispatching further actions, reducers commit state changes based on said actions, state is fed back into views. Suboptimal in the sense that effects (incl. XHR) are generated within a virtual component, rather than in editor/effects, and that the component would have to look at prop changes of state.saving.requesting to infer post saves.

One benefit of data "injection". as seen in §§ One-off, isolated use of site options & Stretching the mechanism too far?, is that it means that the Gutenberg "core" doesn't need to worry about different attribute sources (aside from a pass-through in the serializer, as seen here in the first commit).

Perhaps, if we are to follow the route of § As a virtual data-requesting component, devising some abstraction that could accommodate these new sources—site options, post meta, and others to come—would be a prerequisite.

@aduth
Copy link
Member

aduth commented Oct 23, 2017

This approach will unfortunately not work for non-administrators, who are forbidden from accessing arbitrary site options, even reading them.

forbidden

See: https://core.trac.wordpress.org/ticket/38731

I had thought this was surfaced better in a Gutenberg issue, but I neglected to mention in the related #2759.

We ought to decide how we are going to handle this. It seems either the permissions of the REST API must be expanded, or we need to change our approach to populate these values from the server based on a server-defined block type attribute.

@youknowriad
Copy link
Contributor

What's the status of this PR? Any help needed here? should we close maybe and revisit when we'll work more on customization?

@youknowriad youknowriad added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Feb 2, 2018
@mcsf
Copy link
Contributor Author

mcsf commented Feb 6, 2018

@youknowriad, closing for now sounds good. The branch is actually less interesting than the comments, so when we revisit we should pick up from there instead.

@mcsf mcsf closed this Feb 6, 2018
@mcsf mcsf deleted the add/site-options-attributes branch February 6, 2018 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants