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

Refactor ModalController #1765

Closed
3 tasks done
GuySartorelli opened this issue May 30, 2024 · 1 comment
Closed
3 tasks done

Refactor ModalController #1765

GuySartorelli opened this issue May 30, 2024 · 1 comment

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented May 30, 2024

Currently registering forms from FormFactory classes as form schemas requires applying an extension to ModalController - this should be doable via YAML config directly instead.

Related

Acceptance Criteria

  • ModalController has an API to register form factories via YAML
  • There is a single action for getting a modal form - it accepts a unique identifier for the form as a route parameter, and returns the form (or returns an HTTP error response if no form is available for that identifier)
  • getvars are passed into form factories as needed (this will require either modifying existing factories, modifying the getForm() method signature, or modifying the names of the getvars currently used) Not relevant
  • All existing form factories that are routed through ModalController are registered using the new YAML config instead of as extensions or hardcoded actions

PRs

CMS 5

Reassign to Guy after merging so they can merge up to 6 and rebase

CMS 6

Kitchen sink CI

Note there's currently no documentation about setting up modal forms. There is some limited information about form schema but that's focused more on setting up the REST(ish) endpoints.
Because of that I've opted not to include documentation for this, as it would effectively require adding docs for the JS that results in building the modal on the front-end as well and I think that's a fair amount of work that doesn't feel super in-scope.

Any custom modal forms should continue working exactly as they were (extensions applied to ModalController still work), any customisations to the forms will continue working (the form factories aren't changed at all except for one change to the anchor factory, but that's just changing how it accepts the page ID and doesn't affect the actual form)... this doesn't break anything anyone is likely to be using in any way. I think the only docs we need are the auto-generated "this deprecated method was removed" in the changelog.

That said, if you want docs, let me know the scope of the docs you want and I'll see what I can do.

@GuySartorelli
Copy link
Member Author

GuySartorelli commented Nov 25, 2024

Note that I'm not able to easily make the actions provided by silverstripe/asset-admin/code/Extensions/RemoteFileModalExtension.php part of the generic API in ModalController - but they will keep working just fine as they are.

That has resulted in the scope of this work being limited to the link modal forms used in the WYSIWYG which is the main use of ModalController currently.

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

3 participants