API Make SubsiteXHRController a subclass of AdminController #611
Conversation
GuySartorelli
force-pushed
the
pulls/4/formschema
branch
from
e8e1bde to
18c5cd8
Compare
| /* | ||
| * Reload subsites dropdown when links are processed | ||
| */ | ||
| $('.cms-container .cms-menu-list li a').entwine({ |
There was a problem hiding this comment.
Not only is this selector wrong (should be .cms-menu__list) but this just straight up isn't necessary. Clicking on the links in the left menu reloads the whole left menu anyway.
| $('.cms-container .tab.subsite-model').entwine({ | ||
| onadd(e) { | ||
| $('.cms-container').loadFragment('admin/subsite_xhr', 'SubsiteList'); | ||
| $('#Form_ItemEditForm').entwine({ | ||
| onaftersubmitform(e) { | ||
| // Only load the fragment if this form is the subsite form. | ||
| // We can't add a selector to the form itself so check for the tab we added a specific class to. | ||
| if (this.find('[role="tab"].subsite-model').length > 0) { | ||
| $('.cms-container').loadFragment('admin/subsite_xhr', 'SubsiteList'); | ||
| } | ||
| this._super(e); | ||
| } |
There was a problem hiding this comment.
I think this was trying to reload the selector any time it saw the root.main tab in the subsite edit form.
The selector was wrong, but the approach was wrong too - we should only reload the selector after the form is submitted, not just any time the form loads.
| public function index(HTTPRequest $request): HTTPResponse | ||
| { | ||
| return $this->getResponseNegotiator()->respond($request); | ||
| } |
There was a problem hiding this comment.
This is necessary because we're using loadFragment in js rather than directly hitting the SubsiteList method as a controller action
| return new PjaxResponseNegotiator([ | ||
| 'SubsiteList' => function () { | ||
| return $this->SubsiteList(); | ||
| }, | ||
| ]); |
There was a problem hiding this comment.
That's the only fragment this controller ever cares about.
Using new because that's what's done everywhere that uses it right now and it's not an Injectable class.
| public static function get_template_global_variables() | ||
| { | ||
| return [ | ||
| 'SubsiteSwitchList', | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Generates a list of subsites with the data needed to | ||
| * produce a dropdown site switcher | ||
| * @return SS_List<Subsite> | ||
| */ | ||
| public static function SubsiteSwitchList(): SS_List |
There was a problem hiding this comment.
Since SubsiteXHRController isn't a subclass of LeftAndMain anymore, it doesn't get access to the methods in this extension. This method is only needed in templates anyway so I've just made it always available in templates.
Since I had to break the method signature to make it static I took the liberty of renaming it to make it much clearer what its purpose is.
The alternative would be to make a separate extension with just this logic and apply it to both LeftAndMain and the xhr controller. Happy to do that if you prefer that approach.
| @@ -5,7 +5,8 @@ | |||
| <% include SilverStripe\\Admin\\LeftAndMain_MenuLogo %> | |||
| <% include SilverStripe\\Admin\\LeftAndMain_MenuStatus %> | |||
|
|
|||
| <% if $ListSubsites.Count > 1 %> | |||
| <% if $SubsiteSwitchList.Count > 1 %> | |||
| <% require javascript('silverstripe/subsites:client/dist/js/LeftAndMain_Subsites.js') %> | |||
There was a problem hiding this comment.
Moved the require code here instead of in the method since we don't need to re-require it for the xhr requests and because frankly a require call in an arbitrary PHP method is just gross
GuySartorelli
force-pushed
the
pulls/4/formschema
branch
from
18c5cd8 to
5d38df8
Compare
| */ | ||
| public function canAccess() |
There was a problem hiding this comment.
canAccess() is a non-standard method that's only applied to LeftAndMain classes via the LeftAndMainSubsites extension. It no longer applies here, so we use required_permission_codes instead set to the less restrictive of these since canView() is handling the main access check for us.
GuySartorelli
force-pushed
the
pulls/4/formschema
branch
from
5d38df8 to
7c986c6
Compare
GuySartorelli
force-pushed
the
pulls/4/formschema
branch
2 times, most recently
from
3a401ed to
9e10169
Compare
When a new subsite is added or an existing on is renamed, the subsite selector needs to be reloaded. The code to do this broke at some stage.
GuySartorelli
force-pushed
the
pulls/4/formschema
branch
from
9e10169 to
bb0bbc7
Compare
|
rebased |
When I did silverstripe/silverstripe-admin#1761 I thought
SubsiteXHRControllerneeded form schema logic, but it turns out it doesn't, so it can be a direct subclass ofAdminController.There's two PRs here:
SubsiteXHRControllera subclass ofAdminControllerIssue
LeftAndMaininto its own abstract class silverstripe-admin#1762