silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 27 forks source link

TinyMCE calls not required unless TinyMCE is used #139

Open sunnysideup opened 5 years ago

sunnysideup commented 5 years ago

I am having a look at this:

https://github.com/silverstripe/cwp/blob/master/_config.php#L14-L138

And it appears to me that this is a bit costly. Why run this every time any request (including AJAX!) requests are made. I am not sure if it slows it down in any way, but it seems a bit clumsy. Is there no way we can create a HTMLEditorConfig class and then simply insert this as the default?

What is also missing is any documentation on how to override these settings.

sunnysideup commented 5 years ago

This is what we do:

if (isset($_SERVER['REQUEST_URI']) && strpos($_SERVER['REQUEST_URI'], '/admin/') === 0) {
    //tinyMCE goes here
}

This is a giant hack, but still a lot better than doing all that stuff in the global scope for every request.

robbieaverill commented 5 years ago

One option here is to move this logic into an HTTP middleware class e.g. CwpHtmlEditorConfigMiddleware. This could check whether the path is admin or not before adding the config. It would still run for unrelated admin requests, but not on the frontend at least.

Fluent has a method to detect whether a request is frontend or not, which could be reused or even added to core:

https://github.com/tractorcow/silverstripe-fluent/blob/master/src/Middleware/InitStateMiddleware.php#L57-L81

sunnysideup commented 5 years ago

Fluent has a method to detect whether a request is frontend or not, which could be reused or even added to core

I reckon adding it to core would be great, because we would use it regularly.