Domain config ui #434
Domain config ui #434
Conversation
|
I started to move this into a separate domain_admin_ui module last year, which was abandoned in the end. Something we may need to include is a cache context for the current selected domain, something like this: https://github.com/davidgrayston/domain_admin_ui/blob/8.x-1.x/src/Cache/Context/SelectedDomain.php |
|
Happy to issue a PR to this branch if you feel this cache context is needed |
|
I don't think we need the cache context because the core |
|
Ok, just thought I’d mention as this implementation is switching the config based on a session variable (URL won’t change) |
|
I hadn't considered that. However, I don't think admin forms are cached except by session, so I don't think we have any issues. We could write a test that has two accounts edit the same form to see if caching is an issue. |
|
There are some questions in the TODO here -- 9865bd7 -- that could use review. |
| * @param string $name | ||
| * The configuration name. | ||
| */ | ||
| protected function isAllowedDomainConfig(string $name) { |
There was a problem hiding this comment.
This is PHP7 only code, the string type hint should be removed
There was a problem hiding this comment.
Type hinting (in this form) has been in PHP since version 5
There was a problem hiding this comment.
Most type hints are supported but not 'string' as primitive type. That one is added in php 7.0, see http://php.net/manual/en/migration70.new-features.php
There was a problem hiding this comment.
BTW: I noticed this because I got a hard failure on my php 5.6 setup. So I'm 100% sure ;).
There was a problem hiding this comment.
Ah, was using PHP 7 for so long, I missed that. Fair comment.
There was a problem hiding this comment.
Yeah, we don't type hint strings anywhere else. But oddly this doesn't fail here -- https://travis-ci.org/agentrickard/domain/builds/398266990?utm_source=github_status&utm_medium=notification
|
Module works perfectly btw :) |
The new branch for Domain Config UI.