Added "types" option to list widget #1857
Conversation
|
Deploy preview for netlify-cms-www ready! Built with commit 3ed0efe |
|
This looks awesome! Can't wait to try it out |
|
Awesome, I can't wait to play around with it and see how it all works! My one suggestion is to be able to customize the key used to identify the "widget type". To be a general-purpose modular widget list, it needs to not have hard-coded keys being read from the Front Matter since different SSGs might have a similar concept of modular lists but read the config differently. Basically, there should be an option in the list item config to allow ---
title: Home
layout: home
sections:
- widget: carousel # <-- this key should be configurable, not always `widget`
header: Image Gallery 1
template: carousel.html
images:
- images/image01.png
- images/image02.png
- images/image03.png
- widget: spotlight # <-- this key should be configurable, not always `widget`
header: Spotlight
template: spotlight.html
text: Hello World
- widget: carousel # <-- this key should be configurable, not always `widget`
header: Image Gallery 2
template: carousel.html
images:
- images/image04.png
- images/image05.png
- images/image06.png
---Here's my comment on the other, older PR for reference: #1169 (comment) |
|
I think this is a very elegant solution for Modular Content, thanks @smnh. I checked out the tree and tried it out using the "Home Section" sample field. Here's what I saw: The The widget fields were also missing their configured default values. I also saw variations of the following error message in the browser console: But I also seen it when testing the master branch, so it might not have anything to do with this pull. Finally, I'm wondering how I can review the YAML that's outputted when I save or publish a page. Looking forward to testing this some more. |
|
@cjbrooks12, yes a Here, an example with - label: "Home Section"
name: "sections"
widget: "list"
widgets:
- label: "Carousel"
name: "carousel"
widget: object
widgetField: widget
fields:
...
- label: "Spotlight"
name: "spotlight"
widget: object
widgetField: widget
fields:
...What do you think regarding the name of the @criticalmash, yes the Regarding default values, yes, I've noticed that too, it seems that default values are not working at all. Even when you use production netlify-cms from unpkg.com. As for the preview, you are right, I didn't handle that. I will make the appropriate changes to include widget values in preview. |
|
@smnh Amazing work on this. 😍❤️ It's great that you can introduce such a level of flexibility to NetlifyCMS with what seems to be a relatively small change. What are your thoughts about the UI around adding a new section? I wonder if the two elements is a little confusing. Maybe it could be combined into a single element similar to what was proposed in #1169 |
|
Hi @shaunbent, Yes, I was thinking about making it a single element. But I've tried that Dropdown component that was used in #1169 but for some reason it didn't work. We can always change this UI later thought. |
|
As @cjbrooks12 suggested, I've added a As @criticalmash suggested I've added a way to preview the list and object properties in a much convenient matter by using nested lists. The list and object previews work for all kind of lists, not only one with Here is a full example for the configuration, editor, preview pane and final frontmatter result: collections:
- name: Page
label: page
create: true
extension: md
slug: '{{slug}}'
fields:
- label: "Title"
name: "title"
type: "string"
default: "Hello World"
- label: "Home Section"
name: "sections"
widget: "list"
types:
- label: "Carousel"
name: "carousel"
widget: object
fields:
- {label: Header, name: header, widget: string, default: "Image Gallery"}
- {label: Template, name: template, widget: string, default: "carousel.html"}
- label: Images
name: images
widget: list
field: {label: Image, name: image, widget: image}
- label: "Spotlight"
name: "spotlight"
widget: object
fields:
- {label: Header, name: header, widget: string, default: "Spotlight"}
- {label: Template, name: template, widget: string, default: "spotlight.html"}
- {label: Text, name: text, widget: text, default: "Hello World"}---
title: Hello World
sections:
- header: This is header
images: []
template: This is template
type: carousel
- header: This is Spotlight Section
template: Foo bar
text: Hello World
type: spotlight
---
Following example demonstrates the preview of objects and lists with simple
The configuration for these additional fields is: - label: Simple list
name: simple_list
widget: list
fields:
- {label: Foo, name: foo, widget: string, default: "something"}
- {label: Bar, name: bar, widget: number, default: 3}
- {label: Baz, name: baz, widget: date}
- label: Just an object
name: my_object
widget: object
fields:
- {label: One, name: one, widget: string, default: "Spotlight"}
- {label: Two, name: two, widget: number, default: 4}
- {label: Three, name: three, widget: date}And the stored values are: simple_list:
- baz: 2018-11-07T23:37:15.088Z
foo: My Value
my_object:
one: Value 1
three: 2018-11-08T23:37:52.059Z
two: '5' |
smnh
changed the title
|
Deploy preview for cms-demo ready! Built with commit 3ed0efe |
|
I've been playing around with it and it's working great, this is gonna be so nice! The core functionality works perfectly, customizing the The only issue I've found is that if a list item in the Front Matter doesn't have a Here's my stacktrace, though it doesn't seem too helpful... |
|
@cjbrooks12, yeah, you are right. |
|
@cjbrooks12, I've added error messages for typed lists as you have suggested. The error message is shown both on the control and preview panes. If Check this out: I've also added typed lists to the kitchen sink. |
|
Update is looking great, I've been playing around with it all morning and can't find any more issues. Great job, I can't wait until this gets merged! |
|
Any update on this feature? |
|
Reviewing soon 👍🏼 Sent with GitHawk |
|
Great work on this @smnh, definitely want to see this through to merge. A few takeaways from my first pass:
Taking another pass between today and tomorrow, may add a commit as well, but if you want to tackle these points before that, I'll get your changes reviewed when they come through. @tb don't worry, we'll get this one merged 💪 Thanks again @smnh! |
|
Thanks for taking a time to review this @erquhart |
|
Sounds good 👍🏼👍🏼 Sent with GitHawk |
|
@erquhart, as you have asked I've moved the utility functions from netlify-cms-lib-util.js to a file under netlify-cms-widget-list package (later these functions might be used inside EditorPreviewPane.js), reverted changes to EditorPreviewPane.js and replaced select control and a button with a single dropdown list.
|
|
Awesome work man 👍 I think we should drop the label change for the object widget, folks can add a custom preview if they need that. Question: what happens if one or more type definitions is a widget other than object? Like a string widget that isn't nested in anything? |
You mean this one in
Basically, because typed list must know its types when it is parsing the items, every item must be an object with my_list:
- true # this is boolean, easy to guess
- 4 # this is int number, also easy
- hello world # is this a text, string or maybe a markdown?
- my_image.jpg # even more complicated. looks like an image or a file, but also can be a simple text or stringTherefore, every item in such list must have a To make it work, every item should be a widget of type object with a single field, for example - label: "My List"
name: "my_list"
widget: "list"
types:
- label: "String"
name: "string_type"
widget: object
fields:
- {label: Value, name: value, widget: string, default: "Hello World"}
- label: "Number"
name: "number_type"
widget: object
fields:
- {label: Value, name: value, widget: number, valueType: int, default: 10}
...Later, it can be made even simpler. If one of the types is not an So, one day it can be like this: - label: "My List"
name: "my_list"
widget: "list"
types:
- label: "String"
name: "string_type"
widget: string
default: "Hello World"
- label: "Number"
name: "number_type"
widget: number
valueType: int
default: 10
...That is why I left |
|
Yes, the Because custom widgets can be registered, I don't expect we'll ever be able to infer types with complete accuracy. I'm not suggesting that we allow widgets other than object, I'm suggesting that we drop the |
|
Any update on this? |
- removed utils from netlify-cms-lib-util - removed changes to EditorPreviewPane.js - replaced select with button with dropdown list
…items of object type
|
I've clarified in list widget documentation, that typed lists can only include widgets of type object. |
|
@smnh looking at the demo, typed list values aren't showing in the preview pane, any idea why? |
|
Yeah, because by default preview pane does not show objects properties nor lists values.
|
|
Not sure what you mean, object and list properties do show in the preview pane, just try editing any object or list in the kitchen sink. As far as I understood, those changes you mentioned were adding formatting, not displaying values that were otherwise hidden. |
|
Ok, some clarification. So there were two different issues: First, regarding object and array properties. When I've wrote that preview pane does not show properties ("indexes" in the case of arrays), I've meant that it does not show the property names (or the field names) of the respective values. Although it does show the labels and values of object fields. So if one have a nested object, it does not show to which nested properties and nested objects a value belongs, it only shows the label and the value. In case of the typed array, it becomes more difficult to recognize to which specific item type a value may belong. So to make it clear, I've added lists with property names (for objects) and indexes (for arrays) to the preview. And in case of typed arrays, the type of each item was shown next to its index:
Naturally, this change was made inside Second, because typed array can have different object types, it was necessary to infer the correct object type of each item by its This change also naturally went into Then from your comments I came to a conclusion that all changes to this file should be removed. What I eventually did. I understand now that some of these changes should have stayed. These changes are still available in the commit history. I will try to find some time in the next week or two to refactor it and pull out only what is needed for what I've explained in my the second point. If someone have the time, you are welcome. |
|
Understood - I do disagree on the placement of code, though. I could be wrong, but I'm not certain the concept of "types" needs to be a concern in the CMS core. I'd expect the list widget that implements types to parse said types in its own preview component. Regarding the unfortunately lengthy lifespan of this PR, I want to say how seriously thankful I am for all of the effort that has gone into this! The maintainers of this project don't take contributions lightly, neither the time they represent nor the blocking effect a longstanding PR can have on downstream projects. If you check the repo history, we've been pushing hard to get PR's through with minimal fuss, and increasingly so in the past few months. This PR carries a considerable surface for potential bugs both in code and UX, and represents a shift in the kinds of editing that typically can occur in Netlify CMS, hence the delay. I don't believe my latest change request is superficial, though. We're not talking trivial bits like coding style. Architecturally, it's very important to keep extensions separate from the CMS core. We weren't diligent about this in the beginning and the effort to right the ship was considerable. We as a community, and especially the maintainers, are also responsible to all of the CMS users that will be impacted by any bugs that arise from this feature, impact that will occur whether they want the feature or not. Our lack of tests is regrettable, and contributes heavily to this risk factor. I'll look into pulling down the latest work here and getting previews working so we can merge before Christmas (challenge accepted @watzing 😉). Again thank you @smnh et al for the work, review, and overall push to get this out 💪 |
|
Update: Spent time on this today, and I think the CMS isn't ready for it (architecturally). But you all clearly are 😄. I don't disagree with the aim of the PR at all, I'm more in disagreement with how we're currently processing widgets and values in the editor, and this very awesome feature puts a lot of stress on that exact point. I'm going on break for three weeks starting today, but I don't want to hold this up any longer. I also don't want to roll back to the commit that added unordered list tags to our core. So I propose we merge this as is as a beta feature. We've done this in the past, it puts a disclaimer on the whole thing so we can still change the introduced API without a major release, and there's no guarantee it won't break during usage. That also means previews won't work, so we'll be pending a PR to get that going. But at least you'll have the functionality, and I'll keep an eye out in case a previews PR comes through so it doesn't have to wait longer than necessary. Looking for feedback here. Yay? Nay? |
|
A huge YAY. Thanks for squeezing this in, Shawn! Also massive thanks to @smnh for all your hard work on this, you're a legend. |
|
Yay! As long as it doesn’t break existing functionality. It would be awesome if people can share their experiences on Gitter so we can figure out how to troubleshoot it. I think a beta release is an excellent idea, if we’re confident enough that we can eventually release it as a finished feature. Great work guys! |
|
I agree. If you will merge it now, then later I will create another PR with previews for the typed list values. @erquhart, regarding your comment where the list preview code should live. I agree that it should be inside And the I didn't want to change all that because first, it would be a much bigger change, and second, it might break something because I am not aware of the reasons why it was written this way from the beginning. If you ask me, I think "EditorPreviewPane.js" should be refactored and all the logic specific to different widgets should go to preview components of each widget. @tomrutgers, I took care to make sure that all changes are specific to a case when you add |
|
Merry Christmas :) |
|
Thanks Santa @erquhart , looking forward to having a play with my new toy |
|
Hey guys, Thank you for implementing this! This is awesome and I don't have to do fiddle around with some dirty workarounds in my project. I appreciate it! :) Why didn't you reference this use case in the docs though? Cheers, |
|
@visuallization thanks for all the kind words. It is nice for the other contributors to get the kudos. It might have been an oversight. A PR with the docs would be more than welcome if you would like to take it on. It would be greatly appreciated by the community. You could start by creating the issue first, then when you have time make a PR for the doc changes. |
|
Hi @visuallization, This feature has documentation. You can find this here https://www.netlifycms.org/docs/beta-features/ |
|
Hi everyone, I was really excited to find this thread — the beta feature is working perfectly. Does anyone know if/when this feature will be released? I'm reluctant to use a beta feature outside personal projects. Is anyone using this in production? Many thanks, |
|
Hi @marianneciara. I am using it in production and it's working perfectly. The only reason it's still in beta is because the typed list widget doesn't support custom previews yet. If you're comfortable not using previews for the collection where you want to use the widget, you shouldn't have any problems. |
|
Hi @tomrutgers, that's great news, thanks for the additional context! |
update: I've changed the
widgettotypes, andwidgetitem property totype.The list's "widgets" option allows defining lists of different item types.
Summary
The current list widget does not allow adding items of different types. Assume having a home page with different type sections - "carousel" and "spotlight". Each of these sections can have a different set of fields and both of these sections can be repeated multiple times:
Following frontmatter example demonstrates a
sectionslist with three sections (1. carousel, 2. spotlight, 3. carousel). Every section defines its widget type using thewidgetfield.In order to allow such list to be rendered inside the CMS and to allow adding new items of different types, a new option for
listwidget calledwidgetswas defined. Thewidgetsoption allows defining list of widgets a list could possible have (in other words, array of widgets).Following example demonstrates how a list from the previous example could be defined:
Test plan
Using the above example for field configuration and data from page file the editor renders as following:
To the left of the "Add home section" button there is a
<select>control that allows selecting one of the available widget types that were defined using list'swidgetsoption.Re-ordering, object creation/read/editing/deletion works as expected.
A picture of a cute animal (not mandatory but encouraged)