Modular Content Widget #1169

anthonysapp · 2018-03-12T15:06:19Z

- Summary
The motivation for this pull request is so the CMS can support a front matter property containing a list of modular content or blocks to construct a custom page output.

- Test plan
Add "modular" as widget type in config.yml. This content type accepts a "types" list instead of "fields". Functionality is similar to a list widget with its valueType set to valueTypes.SINGLE. Added the functionality of a dropdown control at the top of the list which chooses the content type to be added to the modular content list. Because it is a variation of the List widget, it retains the ability to re-order child widgets via drag and drop.

Modular content configuration

Modular content widget

- Description for the changelog
Added modular Widget for saving modular content lists to frontmatter.

fixes #1066

verythorough · 2018-03-12T15:08:54Z

Deploy preview for netlify-cms-www ready!

Built with commit 3abf66a

https://deploy-preview-1169--netlify-cms-www.netlify.com

verythorough · 2018-03-12T15:09:24Z

Deploy preview for cms-demo ready!

Built with commit 3abf66a

https://deploy-preview-1169--cms-demo.netlify.com

erquhart

@anthonysapp this is awesome work, does just what it says on the tin! This will be extremely useful for a lot of folks.

I have a rather large change request for you: we should refactor this to be a feature on the existing List widget.

Consider that the list widget is already doing exactly what this does, except you can only define one "type". If a types field is present, we should use that instead of fields and convert the add button to a dropdown. Beyond that, the functionality we need is already there in the List widget.

Let me know if any of that doesn't make sense or if you have questions. Excited to see this get in!

anthonysapp · 2018-03-14T23:15:59Z

@erquhart l will have a look tonight or tomorrow morning, for sure.

Benaiah · 2018-03-15T00:18:49Z

@erquhart What's the use case for your suggestion? Other widgets wrap the List widget for list functionality as well, and introducing this as a feature of the list widget, instead of a wrapper around it, seems kinda weird architecturally.

anthonysapp · 2018-03-15T02:28:20Z

@Benaiah I think maybe because it is a bit of code duplication? My widget is pretty close to the list widget and can probably be merged pretty easily.

anthonysapp · 2018-03-15T16:14:58Z

@erquhart I had a crack at this this morning. I think I've merged all the functionality in the appropriate places. Updated including moving documentation to the list component. The only area I'm not sure if I could make a little cleaner is my modification to the ObjectPreview class.

erquhart · 2018-03-15T19:13:29Z

@Benaiah I don't think use case and architecture map to one another - the use case is a website that needs flexible content authoring either way. I'm not saying combining this into one widget is the best way to do it, but that it might be the best way to do it at the moment for our codebase. I'm not aware of any widget wrapping the list widget, and I don't believe it's currently possible to do so cleanly.

That said, I was thinking that this PR only allowed named field groups to be selected, but it's actually allowing individual fields to be selected by name, which is a big difference, and a noteworthy divergence from the List widget's general role of providing repeatable groups.

anthonysapp · 2018-03-15T19:30:21Z

@erquhart I think this is why I decided against merging it into the List widget in the first place. Nevertheless it was a lot of code duplication so I think your reasoning had merit as well. In any case I merged into List and have all the functionality back as of the latest commit to the PR.

erquhart · 2018-03-15T19:54:25Z

@anthonysapp to help simplify the review, could you do the following?

Re-add dist the .gitignore and remove that directory from the PR
Remove any changes that only affect code style
Remove the hardcoded previewing of specific widgets in the preview component

Let me know if you have questions on that last one.

anthonysapp · 2018-03-15T21:12:16Z

@erquhart Not sure I have any changes that only apply to code style? And yeah a pointer on that last one would help for sure. Thanks!

verythorough · 2018-03-16T01:44:02Z

Ooh, I am very much looking forward to this. I'm pretty sure I'll be able to use it to use Netlify CMS to edit the config.yml file, specifically to make it possible to add new collections via UI.

tech4him1

Just blocking to make sure we don't merge with the dist or package.json changes.

erquhart

Here's a review pointing out the code style changes and a few other things.

Code style changes are changes that only impact how the code looks rather than what it does, like breaking one long line of code down into a few more readable lines. These changes are helpful, but their presence increases the overall risk associated with merging the PR, since the code style changes themselves can break things. Code style changes should be presented in a separate pull request.

erquhart · 2018-03-16T15:54:22Z

package.json

@@ -1,6 +1,6 @@
 {


All changes to this file should be removed.

erquhart · 2018-03-16T16:01:59Z

src/components/EditorWidgets/List/ListControl.js

+    <div {...props} className={`list-item ${ props.className || '' }`}>
+      {props.children}
+    </div>
+  );


Code style change.

erquhart · 2018-03-16T16:02:22Z

src/components/EditorWidgets/List/ListControl.js

-const SortableList = SortableContainer(({ items, renderItem }) => {
-  return <div>{items.map(renderItem)}</div>;
-});
+const SortableList = SortableContainer(({ items, renderItem }) => <div>{items.map(renderItem)}</div>);


Code style change.

erquhart · 2018-03-16T16:02:40Z

src/components/EditorWidgets/List/ListControl.js

+    const listValue = e.target.value
+      .split(',')
+      .map(el => el.trim())
+      .filter(el => el);


Code style change.

erquhart · 2018-03-16T16:03:00Z

src/components/EditorWidgets/List/ListControl.js

    this.setState({ value: valueToString(listValue) });
    this.props.setInactiveStyle();
  };

  handleAdd = (e) => {
    e.preventDefault();
    const { value, onChange } = this.props;
-    const parsedValue = (this.valueType === valueTypes.SINGLE) ? null : Map();
+    const parsedValue = this.valueType === valueTypes.SINGLE ? null : Map();


This line is a code style change.

erquhart · 2018-03-16T16:04:42Z

src/components/EditorWidgets/Object/ObjectControl.js

-      PropTypes.object,
-      PropTypes.bool,
-    ]),
+    value: PropTypes.oneOfType([PropTypes.node, PropTypes.object, PropTypes.bool]),


Code style change.

erquhart · 2018-03-16T16:05:22Z

src/components/EditorWidgets/Object/ObjectControl.js

@@ -90,19 +84,27 @@ export default class ObjectControl extends Component {

  handleCollapseToggle = () => {
    this.setState({ collapsed: !this.state.collapsed });
-  }
+  };


Code style change.

erquhart · 2018-03-16T16:05:37Z

src/components/EditorWidgets/Object/ObjectControl.js

-          { forList ? null : <TopBar collapsed={collapsed} onCollapseToggle={this.handleCollapseToggle} /> }
-          { collapsed ? null : multiFields.map((f, idx) => this.controlFor(f, idx)) }
+          {forList ? null : <TopBar collapsed={collapsed} onCollapseToggle={this.handleCollapseToggle} />}
+          {collapsed ? null : multiFields.map((f, idx) => this.controlFor(f, idx))}


Code style change.

erquhart · 2018-03-16T16:06:40Z

src/components/EditorWidgets/Object/ObjectPreview.js

+  let result = value.get('___name') || value.get(value.get('name'));
+  let valuePreview = null;
+
+  switch (widget) {


This switch statement ties this widget to the widgets below in an undesirable way - the object widget shouldn't know about these other widgets.

erquhart · 2018-03-16T16:07:51Z

website/site/content/docs/widgets/list.md

@@ -8,14 +8,19 @@ type: "widget"



The only changes this file should be additions - changes like switching the list item marker from * to - are code style changes.

anthonysapp · 2018-03-19T15:54:17Z

@erquhart I think I've addressed everything in your request.
The only thing I ran into while getting rid of hard coded references to preview components in ObjectPreview (and the reason I hard-coded the references in the first place) is when there is a user defined preview component that overrides the default preview component (IE in index.html), the user defined component expects an "entry" prop. I wasn't sure how to handle this so for now I've wrapped my resulting component in an error boundary component as described here. If an error is caught, it just tries to output a stringified value of the result. Let me know if you think there's a better solution.

erquhart · 2018-03-19T17:09:19Z

Cool, we'll have to get some eyes on that preview component and consider an alternate approach.

There are still some points from my review that need to be addressed - if you look at the file changes, there are a number of comments still showing. Let me know if you have questions on any of them. Mostly code style changes still remaining, but also .gitignore should have no changes in this PR (currently dist is still being added), and the dist` files that were added to the PR should be removed.

anthonysapp · 2018-03-19T17:55:58Z

@erquhart I had fixed the .gitignore and removed the dist files before the last commit, so not sure why you were still seeing those. Apologies - I think I got to all the code style issues now.

michalmo · 2018-03-21T11:14:37Z

src/components/EditorWidgets/List/ListControl.js

+    const { value, onChange } = this.props;
+    const parsedValue = itemType;
+    this.setState({ itemsCollapsed: this.state.itemsCollapsed.push(false) });
+    onChange((value || List()).push(parsedValue));


one thing I noticed when trying to use this: the whole of the definition from types is written to the output and you end up with widget and label and the other configuration fields in your formatted file.

this is then read in render, and the widget rendering depends on the whole of the type definition being in the the value object.

ideally this would just store the type name and data in the output, and get the type definition from this.props.types based on the stored type name when rendering.

…example modular content settings to config.yml in "Kitchen Sink" post for demo project

morrislaptop · 2018-04-21T14:24:44Z

Can't wait for this!

erquhart · 2018-05-25T15:37:16Z

Hey @anthonysapp just checking in - still planning to circle back on the issues raised in the latest comments?

anthonysapp · 2018-05-25T15:50:28Z

Hey all this is some great discussion. So sorry to have been unresponsive. I'm quite busy at the moment on another project but I would indeed like to loop back to this in the next week or so, as I'd love to use it for an upcoming project myself :).
If anyone else feels the urge to help out, by all means.

shaunbent · 2018-05-29T07:45:17Z

Awesome work on this @anthonysapp - I need this exact feature right now. I've sort of managed to recreate it using registerEditorComponent but its not the best user experience and it also means might front end is a little heavier as I need to transform my Markdown AST to React on the client.

This PR/feature would completely solve my problem. I really hope this makes it in soon.

anthonysapp · 2018-06-01T18:34:56Z

@erquhart @cjbrooks12 hi guys, I took a look at this last night and I'm just curious if you both think this should still be merged with the List widget, or you think it warrants its own widget with the proposed changes? Also - I'm not 100% sure I understand this comment. What is inherently wrong with using the "name" field to reference each component? I know I'm probably missing something easy :)

cjbrooks12 · 2018-06-01T19:02:35Z

Technically-speaking, I would see the List widget as a special case of the Modular widget where there is only one available type. I think there is still a case for keeping the Modular widget separate for clarity of intent.

As for the practical reason for wanting to customize the field used as the key, it is simply that the consumers of these modular blocks may consume them differently than Netlify CMS does.

For example, I'm build a SSG with this kind of modularity baked into the framework itself, and Orchid reads these blocks from the type field, not name. See here for an example: the components top-level field is the modular container, and each list item is a modular block identified by its type. Orchid wouldn't be able to understand the output of this widget if it only worked with name.

Or take Forestry's Blocks, which uses template to identify the type of the modular item. If someone were to migrate to Netlify CMS from Forestry, wouldn't it be easier to tell the Netlify CMS widget to look for the template parameter rather than finding and changing all instances of template with name and hoping you got it all right?

As far as making it happen in the code, it shouldn't be much of a change. Rather than doing

item.name

to get the modular type, it would just be one step back with a configuration value to get it instead:

// config.typeKey defaults to 'name' but is configurable by the end-user
item[config.typeKey]

anthonysapp · 2018-06-01T19:11:23Z

@cjbrooks12 I think I'm following now. Thanks for the feedback!

gil-- · 2018-06-12T02:37:09Z

Just thought I'd say that this is an extremely exciting widget and will really elevate the possibilities (custom layouts on a per page basis). 💯

Looking forward to seeing it shipped.

LoveYouFyi · 2018-06-27T19:11:53Z

I'm monitoring the status of this modular content widget as it is all I'm waiting for to jump into Netlify CMS.

erquhart · 2018-07-31T21:59:02Z

Closing as stale - if someone wants to pick it up, please comment.

airtonix · 2018-08-11T05:49:35Z

it would be great if we could use a group of other collections in place of types. This way, existing blocks are re-usable.

DjangoCMS and EPiServer calls these blocks.

And if you have a collection of blocks that are actually layout types with further modular widgets in them, you can do awesome things.

Perhaps the widget should be called include or content-area or blocks, a field that lets me include one or more other collection items from one or more other collections.

criticalmash · 2018-08-24T14:30:11Z

What will it take to get this past the finish line? Are there particular features that still need to be implemented? or tests to pass?

I'm willing to help out, but I have zero React experience.

loremipson · 2018-08-24T16:58:54Z

Hey, our team is currently exploring some ideas that are very similar on our own fork. We'll give an update when we feel more confident in our approach here.

erquhart · 2018-08-27T20:25:19Z

@criticalmash things are pretty well laid out in the comments above, just a matter of someone putting together a good solution.

@loremipson sounds good 🤞

mittalyashu · 2018-09-12T17:58:35Z

As discussed in the NetlifyCMS community meeting.

I was trying to say this that it can split the .json file (each object) into different card.

alexanderfountain · 2018-09-13T13:39:43Z

This sounds awesome. I have been unable to get this pull working at all with current version of netlifycms though.

chan360 · 2018-10-07T20:47:00Z

This sounds like a must have widget when it comes to customizability and I'm wondering if there has been new updates on this?

erquhart · 2018-10-26T18:58:06Z

No updates, still needs a sponsor.

erquhart · 2018-11-06T18:02:50Z

For those watching this issue, a new implementation just popped up courtesy of @smnh in #1857 - we need some reviewing and testing, please check it out and make comments!

erquhart suggested changes Mar 14, 2018

View reviewed changes

cjbrooks12 mentioned this pull request Mar 16, 2018

Integrate Netlify CMS modular widget to manage Components and Menus orchidhq/Orchid#96

Closed

tech4him1 suggested changes Mar 16, 2018

View reviewed changes

erquhart suggested changes Mar 16, 2018

View reviewed changes

michalmo reviewed Mar 21, 2018

View reviewed changes

aleksandarbasara mentioned this pull request Mar 26, 2018

Customize markdown editor UI? #1194

Closed

Anthony Sapp added 10 commits March 28, 2018 14:58

no message

da66885

fixed preview widget

7b5ad61

no message

2a42bf4

removed console.log

b6dc35a

chnaged back to types structure

63bed40

added dist

dee22ce

no message

90b576a

updated docs

be91492

added contributor, removed unnecessary componentDidMount

2e0371d

fixed dropdown z-index, added ModularContentPreview component, added …

aadc15d

…example modular content settings to config.yml in "Kitchen Sink" post for demo project

erquhart mentioned this pull request Apr 25, 2018

Netlify CMS 2.0 #1280

Closed

10 tasks

tech4him1 mentioned this pull request May 14, 2018

Feature Request: Switch Widget #1354

Closed

loremipson mentioned this pull request May 22, 2018

CMS visual builder (config.yml generator) #1167

Open

erquhart added status: 1-design-review area: extensions/widgets labels May 25, 2018

cjbrooks12 mentioned this pull request Jun 6, 2018

No debug info on backend crash orchidhq/Orchid#133

Closed

loremipson mentioned this pull request Jun 7, 2018

Drag and drop builder #1401

Closed

erquhart closed this Jul 31, 2018

tech4him1 added the status: stale label Aug 7, 2018

tech4him1 mentioned this pull request Aug 8, 2018

Flexible Layouts Custom Widget #1066

Closed

erquhart mentioned this pull request Nov 6, 2018

Added "types" option to list widget #1857

Merged

Modular Content Widget #1169

Modular Content Widget #1169

Conversation

anthonysapp commented Mar 12, 2018 • edited Loading

verythorough commented Mar 12, 2018 • edited Loading

verythorough commented Mar 12, 2018 • edited Loading

erquhart left a comment

Choose a reason for hiding this comment

anthonysapp commented Mar 14, 2018

Benaiah commented Mar 15, 2018

anthonysapp commented Mar 15, 2018

anthonysapp commented Mar 15, 2018 • edited Loading

erquhart commented Mar 15, 2018 • edited Loading

anthonysapp commented Mar 15, 2018 • edited Loading

erquhart commented Mar 15, 2018

anthonysapp commented Mar 15, 2018

verythorough commented Mar 16, 2018

tech4him1 left a comment

Choose a reason for hiding this comment

erquhart left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonysapp commented Mar 19, 2018 • edited Loading

erquhart commented Mar 19, 2018

anthonysapp commented Mar 19, 2018

Choose a reason for hiding this comment

morrislaptop commented Apr 21, 2018

erquhart commented May 25, 2018

anthonysapp commented May 25, 2018

shaunbent commented May 29, 2018

anthonysapp commented Jun 1, 2018

cjbrooks12 commented Jun 1, 2018 • edited Loading

anthonysapp commented Jun 1, 2018

gil-- commented Jun 12, 2018 • edited Loading

LoveYouFyi commented Jun 27, 2018 • edited Loading

erquhart commented Jul 31, 2018

airtonix commented Aug 11, 2018 • edited Loading

criticalmash commented Aug 24, 2018

loremipson commented Aug 24, 2018

erquhart commented Aug 27, 2018

mittalyashu commented Sep 12, 2018 • edited Loading

alexanderfountain commented Sep 13, 2018

chan360 commented Oct 7, 2018

erquhart commented Oct 26, 2018

erquhart commented Nov 6, 2018

anthonysapp commented Mar 12, 2018 •

edited

Loading

verythorough commented Mar 12, 2018 •

edited

Loading

verythorough commented Mar 12, 2018 •

edited

Loading

anthonysapp commented Mar 15, 2018 •

edited

Loading

erquhart commented Mar 15, 2018 •

edited

Loading

anthonysapp commented Mar 15, 2018 •

edited

Loading

erquhart left a comment •

edited

Loading

anthonysapp commented Mar 19, 2018 •

edited

Loading

cjbrooks12 commented Jun 1, 2018 •

edited

Loading

gil-- commented Jun 12, 2018 •

edited

Loading

LoveYouFyi commented Jun 27, 2018 •

edited

Loading

airtonix commented Aug 11, 2018 •

edited

Loading

mittalyashu commented Sep 12, 2018 •

edited

Loading