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

TinyMCE calls not required unless TinyMCE is used #139

Open
sunnysideup opened this issue Jul 11, 2018 · 3 comments
Open

TinyMCE calls not required unless TinyMCE is used #139

sunnysideup opened this issue Jul 11, 2018 · 3 comments

Comments

@sunnysideup
Copy link

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
Copy link
Author

sunnysideup commented Jul 11, 2018

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
Copy link
Contributor

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
Copy link
Author

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.

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

4 participants