feat(object): variable types

mmkal commented 5 months ago

Summary

Allow for objects to use variable types - effectively enabling "dependent fields" for objects. Also, improve support for objects with required: false - use list widget for those too, so that you can add remove the whole object from the CMS, depending on whether the content author wants it to be defined or not.

Implementation: use the list widget control for objects when either types is defined, or when required=false.

Replaces #3891 Goes some of the way to addressing #565 (there's a limitation - dependent fields must be nested under an object or list).

Test plan

Added the typed_object field to the "kitchen sink" collection and tried it out.

Here's how it renders:

https://github.com/decaporg/decap-cms/assets/15040698/d98f7636-eba0-4f43-8f0b-9631b7dba7ee

Help needed

Code organisation: I don't like that the object widget is now actually defined in the decap-cms-widget-list package. The decap-cms-widget-object package is now reduced to a dependency of the list widget. To be honest I don't really see the value in splitting the widgets out into separate packages anyway, but that's another story. It was a minimal way of getting the change in. Maybe a refactor to merge the decap-cms-widget-list and decap-cms-widget-object packages could be a follow-up.

Naming: similarly, the helper which creates the widget is called DecapCmsWidgetList.ObjectWidget() but that's kind of confusing. It's the object widget. So maybe the following refactor would make sense:

Rename the decap-cms-widget-object package to decap-cms-widget-struct - it's just a general purpose control component for rendering the fields for an dictionary-like field (either object or list). It doesn't correspond to an actual widget anymore
Create a new package called decap-cms-widget-object-and-list which depends on both decap-cms-widget-list and decap-cms-widget-struct. This new package is just a thin wrapper which decides whether to use the list control or the struct control.

Testing: I tested it manually, but noticed there's are HTML files dev-test/index.html and dev-test/backends/test/index.html which contain JSON-serialized values of kitchen sink collection sample files. Am I supposed to add to those by hand?

@martinjagodic do you know who can help me answer some of these questions?

Next steps

Stuff I don't really want to do as part of this PR

Part 2 of the recommendation I made in https://github.com/decaporg/decap-cms/pull/3891#issuecomment-1887545190 - right now, you can't have variable types at the top level of an entry. I started looking into this but it's fiddly.
Some UI tweaks - it probably shouldn't say "0 typed object" before you've added the value
The refactor described above
A bigger refactor which gets rid of all the decap-cms-widget-* packages and just puts them in a subfolder inside decap-cms-core. I can't really see the downside of that as an end goal, but obviously it'd be a fair amount of work to actually do that, and a fair amount of work to get buy in from the maintenance team...

Checklist

[x] I have read the contribution guidelines.

netlify[bot] commented 5 months ago

Deploy Preview for decap-www ready!

Name	Link
Latest commit	bc2591303d3db4bef269bddbdbafa752734c0ef8
Latest deploy log	https://app.netlify.com/sites/decap-www/deploys/65a16c21f713b50008a7d3a6
Deploy Preview	https://deploy-preview-7031--decap-www.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

martinjagodic commented 5 months ago

@mmkal this looks great. @demshy could you look into this and answer the questions?

To me, the decap-cms-widget-object-and-list and decap-cms-widget-struct are a bit confusing. Wouldn't it be simpler to have one package for both list and object instead of three?

Joining widget packages: we agree with this! But it would be a big effort for little effect (user-wise), so it wasn't very close on our radar. We would have to choose which to do first: remove immutable.js or join packages. We can't do both at the same time.

mmkal commented 5 months ago

Good to hear - if you're open to joining packages, maybe it could start with just the list and object widgets moving to decap-cms-core for now. IMO it'd make sense to do that before moving off immutablejs - it's easier to do and centralizing generally would probably make a big dependency change lower lift.

decaporg / decap-cms