silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 92 forks source link

Increase getSchemaRequested/getSchemaResponse to public methods #240

Open robbieaverill opened 7 years ago

robbieaverill commented 7 years ago

Controllers that want to use the new React form builder style APIs can use LeftAndMain::getSchemaRequested and ::getSchemaResponse to help with the client app communication.

Extensions to LeftAndMain controllers can't though because they're protected methods.

As a result, you see examples like this happening instead.

At this stage I can only see asset-admin will be affected by this in core packages.

Related issues

chillu commented 7 years ago

What use case needs Extension rather than extending a base controller? I can see the need in ModalController, but what else is this blocking at the moment?

robbieaverill commented 7 years ago

So the use case I have at the moment is content review, which adds some functionality to CMSPageEditController via a LeftAndMainExtension.

I suppose that two immediate possibilities that don't involve changing the visibility in LeftAndMain could be to increase the visibility in the LeftAndMainExtension instead, or to use entirely separate controllers instead of extending the CMS controller.

tractorcow commented 7 years ago

I'd rather move it out of LeftAndMain altogether. What about putting it in FormSchema?

tractorcow commented 7 years ago

getSchemaRequested is 2 lines; I'm not worried so much if someone has to inline this somewhere.

tractorcow commented 6 years ago

This logic needs to be shifted out of LeftAndMain, and I would prefer to put it into LeftAndMainFormRequestHandler which is where all form-schema response handling should be done.

The forms themselves are the natural delegates for handling their own schema.