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

RFC: New React based replacement for GridFieldDetailForm #590

Open
robbieaverill opened this issue Aug 2, 2018 · 6 comments
Open

RFC: New React based replacement for GridFieldDetailForm #590

robbieaverill opened this issue Aug 2, 2018 · 6 comments

Comments

@robbieaverill
Copy link
Contributor

robbieaverill commented Aug 2, 2018

Affected Version

SilverStripe 4.x

Description

Coming from a GridField world

In SilverStripe 3.x, a developer could easily use GridField to scaffold a mechanism for editing a has_many/many_many list of data models (for example, on a page). This would provide a few features for free such as an edit form and a delete/unlink handler.

In SilverStripe 4.x we will be introducing React GridField, so this kind of situation will soon be viable in a React powered world as well (#556).

That said, in SilverStripe 4.x there isn't necessarily a good reason to use GridField to power interfaces like this any more (besides the additional functionality that GridField provides). One example is switching silverstripe-elemental to use a React/GraphQL driven editor UI from a PHP GridField.

Moving away from GridField

The problem with this issue is that once we remove GridField we also lose the built in edit form. Deleting, unlinking and other inline operations are easy enough to achieve with GraphQL mutations, but the edit form does many things; render on a standalone page in the CMS, render a form, render a field list of form fields. At this stage there isn't a drop in replacement for this, and this means that in situations like this we're going to continue to power the feature with GridField, while changing the UI entirely to be a bespoke mini React app.

Options

1. Wait for React GridField (#556) and tie that in instead

Presumably the React GridField will also come with an equivalent of the GridFieldDetailForm which does this.

Pros: the feature is already in development and will work out of the box, especially when ModelAdmins for example are all using this

Cons: it's still using GridField for the same reasons it did in SilverStripe 3.x (convenience)

2. Create something standalone for this

Noting because it was an option, but not a very good one (reusability, etc)

3. A centralised place for rendering FormBuilderLoader

Perhaps there could be something like /admin/edit/:classname/:ID which is a React controlled area that renders an edit form via FormBuilderLoader, including scaffolding all the CMS tabs etc. Designed to be entirely abstract, presumably you could edit any data model that you had permissions for...

It would need to allow extensibility for things like breadcrumbs, the back link, etc.

Would not be available as a CMS menu item.

Pros: centralised, reusable, React GridField could use it as well for the GridFieldDetailForm replacement

Cons: ?


It's also worth mentioning that at some point in the future, the CMS area itself will be React powered as well. At a high level, this will require the same thing as this - a React driven form editing area for pages.

I don't have a huge amount of ideas here, but thought I'd kick this off for discussion to start with.

@sminnee
Copy link
Member

sminnee commented Aug 2, 2018

Could you elaborate on the specific case where you’re wanting to make use of this? It would help to understand the implications of different approaches.

@robbieaverill
Copy link
Contributor Author

Yeah sure. If I’m using a form within a react app I can use FormBuilderLoader to render a new view with form fields in it, but if the react app is within CMS form fields (shivved with entwine) then I don’t have the option to do a full screen edit form any more. The specific use case right now is the content blocks editor- it will have inline editing in the react app for some block types, but complex block types like the user forms block will need to link out to a standalone edit form, which so far has been a PHP GridFieldDetailForm. We want to remove GridField from this editor, but this is the part that we have no immediate React replacement for.

@chillu
Copy link
Member

chillu commented Aug 2, 2018

  1. Wait for React GridField and tie that in instead

That's just going to solved through "3. A centralised place for rendering FormBuilderLoader" eventually as well. We haven't thought about detail editing as part of the React GridField work yet, but I see this as an independent feature. So it can either happen before or after React GridField.

  1. A centralised place for rendering FormBuilderLoader

I think this is the way to go, but there's a few things to work through. We've already started down this path through Form Factory which is used in AssetAdmin.

  • Pro: More alignment with API-driven UI: If you have one editing UI for each model separate from views/routes and "container forms", it's much easier to reuse that. For example, if you want to extend TagField with "detail edit" buttons on each tag, you don't need to build your own Form, and can just link to an existing view instead
  • Pro: Less UI variations make it easier to cache form schemas on the client, and speed up user experience. By knowing about all forms upfront, we could even pre-generate all form schemas and send them through with the initial request.
  • Pro: Access control no longer handled through LeftAndMain controller. This forces us to centralise access control, rather than having it splattered between different layers (LeftAndMain->init() vs. GridField vs. ModelAdmin vs. GridFieldDetailForm)
  • Con: No more implied access control through form field presence. The getCMSFields() method has been "abused" to limit access by conditional field rendering. Which extends to the ability to view and save records through GridFieldDetailForm. We should enforce this via can*() permissions anyway. Because we don't have a declared "entry point" in getCMSFields(), every DataObject will need to have a route to an editing UI and centralised save operations.
  • Con: Context-based view variations get harder. You can't setEditDetailForm() for a specific GridField instance, which e.g. shows more limited fields. This is a bit of an anti-pattern anyway, and should be solved through user permissions rather than editing context.
  • Con: Context-based behaviour variations get harder. Adding a new record to an existing many-many relationship will need some thought when there's just one generic edit form.

Note that there's a related discussion about Form Submissions using GraphQL. The eventual goal should be a 100% API driven CMS UI, with as much alignment as possible to a "GraphQL Content API" which covers the entire CRUD surface. This would require us to further decouple save operations, by treating forms as a view layer, but saving records through mutations instead. That's hard since there's often not a 1:1 match between form fields and record fields, and form fields themselves validate and transform data (most of which would be more cleanly handled on the model layer). I think we can move towards this model by starting to decouple record editing from GridFieldDetailForm, without solving the entire "forms vs. mutations" issue.

@sminnee
Copy link
Member

sminnee commented Aug 2, 2018

I don't know enough about the React stack to recommend a specific solution — I'll leave that to Ingo — but I want to share some principles that I think we should follow:

  • Replicating the SS3/Entwine approach should not be our starting point for architecture decisions. Instead we should be saying "how would a developer without deep knowledge of our history build this?" Specifically we should aim to build our systems in a way that would make some sense to a React developer newly introduced to our project.

  • SilverStripe does have some special powers that distinguish it from other apps. Specifically, it's designed to build UIs that will adapt themselves to the data model they are provided with. Right now we do this by having the server provide a FormSchema that maps out how the UI should interact with the data model. I would hope that this doesn't stop SS4-React development from being intelligible to a React developer.

  • But there is a risk. FormSchema, if stretched too far, turns into "UI definition on the server". This is what we did in SS3 but I don't think that this would be seen as a sensible architecture by any React developer. The distinction is subtle — by way of analogy, it's similar to "how much logic is healthy to go into templates?" — but FormSchema's should be providing declarative information enough information about the content/data being managed for the UI to make sensible decisions about how to adjust itself. We do provide a UI construction tool in our toolbox, it's called React.

  • The warning heuristic for using FormSchema as a UI-construction toolkit is the presence of many nested fields. Categorising your fields into groups makes sense, but deciding whether those should be laid out in tabs, accordions or subheadings doesn't. Indicating primary, secondary, and advanced actions on the server makes a lot of sense, but deciding to put those in a pop-up menu doesn't. It is a blurry line where we have to be pragmatic and we want to avoid forcing website developers to jump into JS customisation for simple changes. But that doesn't mean we wouldn't benefit form some principles/heuristics here.

  • GridField is a particularly pernicious part of the SS3 stack for allowing complex UI definition on the server, and why I would expect "React GridField" to be radically simpler.

  • I don't see it as self-evident that a React GridField should have support for hosting subordinate URLs such as detail forms. We use a routing engine to define URLs that are mapped to React views; we can use this to provide detail forms, etc, as well, and the GridFields would just provide a links/buttons to visit it.

  • If we are going to introduce hacks, we should introduce hacks in the way that SS3 & Entwine pieces play with React, and not how we approach the definition/configuration of React UIs.

@chillu
Copy link
Member

chillu commented Aug 2, 2018

100% on what Sam said, thanks for outlining that without getting bogged down in details :)

We do provide a UI construction tool in our toolbox, it's called React.

The challenge over the next year will be to make that a practical reality: Customising form presentation and behaviour via React in SS4 is a lot more involved. Compare the Toggle a form field how to with the PHP required to make this work in the old style:

function getCMSFields() {
	$fields = parent::getCMSFields();

	if ($this->Country == 'US') {
		$fields->addFieldsToTab('Root.Content',
			new TextField('State')
		);
	}
}

Or via pseudo Entwine (I'm a bit rusty there):

$('#Form_EditForm field#State').onmatch(function() {
	this.toggle(this.parent('form').find('field#Country').val() == 'US');
})

I'm not saying we should revert to server-side UI definitions, but you can see how devs will tend towards the option with the least amount of friction :)

@sminnee
Copy link
Member

sminnee commented Aug 3, 2018

The challenge over the next year will be to make that a practical reality.

It might be useful to flesh out a bit of a playbook for how we imagine people doing this, at a high-level, for different situations.

For the first step, would this best go into a separate RFC, a docs PR, or something else?

PS, for your specific example I was imagining something like this, powered by a react-based displaylogic module:

"State": {
  type: "text",
  category: "Content",
  "showif": "Country = 'US'",
}

I personally like the idea of using pegjs to build a simple DSL for the showif values (I've literally implemented one in a lightning talk), but Ingo might give me the Skeptical Eyebrow about that one. ;)

Update: no Skeptical Eyebrow needed, someone else has done this: https://www.npmjs.com/package/@smartorigin/smart-filter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants