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

Add inline editing capability to the React ElementEditor for top level elements #295

Closed
4 tasks
robbieaverill opened this issue Aug 6, 2018 · 4 comments
Closed
4 tasks

Comments

@robbieaverill
Copy link
Contributor

robbieaverill commented Aug 6, 2018

Top level elements should be able to be edited inline in the ElementEditor React app, within the context of a page's CMS fields.

In the future this will include some level of nesting, e.g. layout blocks or lists of blocks.

Considerations:

  • we need to ensure that the core form schema + FormBuilderLoader will allow us to have nested forms, e.g. return the form field lists without the associated form tags, since it'll be rendered inside the Page's form and nested forms aren't valid
  • ensure that saving the page will also save all of the FormBuilderLoader data for top level nested elements on the page (this is not a React driven action)
  • ensure that if a React change tracker exists in the React form and is used, that it works with the jQuery change tracker on the page - changing content in an element's field and navigating away from the page should trigger a change tracker warning (Alert for unsaved block changes #291)
  • adding save/publish etc actions to individual Elements in the editor is a different story (Save or publish blocks individually #290)
  • GridFields won't work (no React component yet) (RFC: React/GraphQL GridField silverstripe-admin#556)

Tasks:

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Aug 14, 2018

we need to ensure that the core form schema + FormBuilderLoader will allow us to have nested forms, e.g. return the form field lists without the associated form tags, since it'll be rendered inside the Page's form and nested forms aren't valid

We can override the injector definition for Form (in admin) with our own component that does not actually render a form tag. We would have to re-implement some things like messages but considering it's quite a custom form this might make sense.

Field names are likely to have conflicts but redux-form provides a solution by using FormSections. Although we may not find this necessary - fields with the same names is valid HTML.

ensure that saving the page will also save all of the FormBuilderLoader data for top level nested elements on the page (this is not a React driven action)

There's an example on the redux-form website for submitting redux forms from separate unrelated react components. We can entwine this into the existing entwine stuff 😉 . We will have to define our own redux form as we need to specify a name in the config, but it's a relatively simple component that we can override and inject. Alternatively, it might be a good idea to update admin to have form names within the redux form config.

ensure that if a React change tracker exists in the React form and is used, that it works with the jQuery change tracker on the page - changing content in an element's field and navigating away from the page should trigger a change tracker warning (#291)

React change tracker exists and works separately from the entwine version. It's trivial to adjust the settings for the entwine version, but leaving it makes sense so it can continue to adjust the state of the save button.

The react version is in two parts. One part is bound to the actions (buttons) and updates them if it tracks any state changes. The other part is part of the router and observes changes on redux forms. Considering we can't use a router for this, the react version won't be giving us a warning.

GridFields won't work (no React component yet) (silverstripe/silverstripe-admin#556)

We should be removing GridField (touchwood). We will need silverstripe/silverstripe-admin#590 to fully remove it though.

Submitting the form and saving with a mutation is a little more complex. FormBuilderLoader is already set up to handle the REST way of doing things by default.

@robbieaverill
Copy link
Contributor Author

We should be removing GridField (touchwood)

Sorry, I mean that user defined CMS fields for their content blocks could be a GridField and won't work in this situation

Nice write up - looks good!

Submitting the form and saving with a mutation is a little more complex. FormBuilderLoader is already set up to handle the REST way of doing things by default.

We might need to start work on a new FormBuilderLoader or modify the old one to allow for GraphQL mutation HOCs instead. That is the direction that core will be heading in anyway

@ScopeyNZ
Copy link
Contributor

Sorry, I mean that user defined CMS fields for their content blocks could be a GridField and won't work in this situation

Right - we'll have to wait for silverstripe/silverstripe-admin#556 AND silverstripe/silverstripe-admin#590 to completely detach ourselves from GridField.

Additionally, block authors who add GridFields to getCMSFields will have to mark their custom blocks as not in-line editable (more here: #266) but I think that's fine.

@ScopeyNZ
Copy link
Contributor

Closing this issue. I've raised individual tasks to resolve this - noted in the original post.

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

2 participants