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

Add context switch mechanism in core #14712

Open
sdrenth opened this issue Sep 3, 2019 · 12 comments
Open

Add context switch mechanism in core #14712

sdrenth opened this issue Sep 3, 2019 · 12 comments
Assignees
Labels
blocked Active participation around the pull request or issue required. Consensus is not reached. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.

Comments

@sdrenth
Copy link
Contributor

sdrenth commented Sep 3, 2019

Feature request

Summary

The power of MODX lies in the ability to have multiple contexts in the same CMS. But isn't it weird that the core does not provide a way to switch between those multiple contexts?

In my opinion this context switch mechanism should be included in the core without having to do some extra coding.

Why is it needed?

I think it should be included in the core because the power of MODX lies in the support of multiple contexts and that it would be a nice enhancement.

Suggested solution(s)

I think the context switching mechanism could be added in the index in the webroot and perhaps include a config file (/core/config/config.inc.php) which could contain a domain name => context mapping.

@Mark-H
Copy link
Collaborator

Mark-H commented Sep 3, 2019

What exactly would a core context router do, technically?

Given we have routers available as extras based on domains, domain aliases, base url, domains and base urls, browser language, and maybe even more.. giving the responsibility for determining the context to extras is one of the more flexible parts of MODX.

What would a core implementation be able of accomplishing that an extra can't?

@sdrenth
Copy link
Contributor Author

sdrenth commented Sep 3, 2019

@Mark-H At least provide a default mechanism to switch between contexts based on hosts/base url's. In the ideal situation you would also be able to customize the code and return the context key which you want to initialize, so you could also do stuff based on browser language etc.

It's not about what an extra can't, but I think it should be part of the core because its a basic and vital part of MODX.

I'd like to hear some more opinions about this.

@JoshuaLuckers JoshuaLuckers added proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. state/being-discussed labels Sep 3, 2019
@JoshuaLuckers
Copy link
Contributor

Related to and/or duplicate of issue #14462

@Mark-H
Copy link
Collaborator

Mark-H commented Sep 3, 2019

In the ideal situation you would also be able to customize the code and return the context key which you want to initialize, so you could also do stuff based on browser language etc.

Do you mean you want to make it possible for an extra to handle the logic for what context to initialise... like the current situation? ;)

With some MODX principles in mind I don't see what principle would benefit from turning this into a core feature.

I do appreciate some people's argument to want to put all all things into the core, but we benefit much more from the flexibility of extras. I like the analogy to rich text editors. 97% of sites use a RTE (SiteDash data, just now), but if a decade ago TinyMCE was built into the core, we may not have had Redactor, TinyMCE RTE, TinyMCE Wrapper, CKEditor, etc. If ContextRouter (which does the domain > context routing you propose should be possible in the config.inc.php) was built into the core 5 years ago, xRouting may not have been made as a more powerful improvement.

Maybe (big maybe) the performance principle would benefit if core doesn't have to init the web context when the request is for another one. However, rather than incorporating a small subset of existing extra functionality into the core, such a performance benefit could also be achieved with a new event that fires in modX.initialize before the call to _initContext that can override the $contextKey. That sort of matches what you propose in your last post - but without the default domain-based logic in the core.

@sdrenth
Copy link
Contributor Author

sdrenth commented Sep 3, 2019

@Mark-H I understand you'd like to keep the core clean and minimised and I understand why, but still I think this should be part of the core because it's part of the base principle of MODX in my opinion (because of multi context support).

I can understand why TinyCME/Redactor should be kept as extra's and should not be integrated within the core because of the flexibility aspect. But context switching is something which you should be able to do out-of-the-box in my opinion.

I think it's weird that everything in MODX is setup for using multiple contexts, but if you want to actually switch between them that developers have to install an extra for it.

If ContextRouter (which does the domain > context routing you propose should be possible in the config.inc.php) was built into the core 5 years ago, xRouting may not have been made as a more powerful improvement.

It could also mean that the context switch mechanism in the core would be more involved and no new extra was needed ;)

Maybe (big maybe) the performance principle would benefit if core doesn't have to init the web context when the request is for another one. However, rather than incorporating a small subset of existing extra functionality into the core, such a performance benefit could also be achieved with a new event that fires in modX.initialize before the call to _initContext that can override the $contextKey. That sort of matches what you propose in your last post - but without the default domain-based logic in the core.

The performance benefit is also of importance, so if there would be such a system event it would be a good improvement in my opinion.

@wokenlex
Copy link

wokenlex commented Sep 4, 2019

Symfony, for example, can switch locales from route config and configuration is very easy.
Why modx cant do something like that? When i used contexts in modx - i should to make the locale manager, and i think every who using it - did it too.

@Mark-H
Copy link
Collaborator

Mark-H commented Sep 4, 2019

I don't understand that comment in relation to the discussion here @wkwkgit, but you can set a locale context setting to change locales per contexts.

@wokenlex
Copy link

wokenlex commented Sep 4, 2019

It has direct relation. I mean not the setting locale to context, i'am about routing all of it.

Example:

 context_name_one:
     path:       /
     host:       en.example.com
     locale: en
 context_name_two:
     path:       /
     host:       es.example.com
     locale: es

But i have to implement the context routing every time, when i using Revo. But the contexts - is a only one feature, why someone moved from evo to revo, so its very strange that its look like not complete feature.

@JoshuaLuckers
Copy link
Contributor

From the documentation:

Contexts allow MODX configuration settings to be overridden, extended, isolated, or shared across domains, sub-domains, sub-sites, multi-sites, cultural-specific sections, specific web applications, and so on.

As @Mark-H said earlier:

[..] However, rather than incorporating a small subset of existing extra functionality into the core, such a performance benefit could also be achieved with a new event that fires in modX.initialize before the call to _initContext that can override the $contextKey. That sort of matches what you propose in your last post - but without the default domain-based logic in the core.

@JoshuaLuckers
Copy link
Contributor

@sergant210
Copy link
Collaborator

I agree with @sdrenth . It should be in the core. So that new MODX users do not need to install additional extras.

The compromise is to create a system setting that disables this functionality.

and perhaps include a config file (/core/config/config.inc.php) which could contain a domain name => context mapping

It can be done automatically.

@ilyautkin
Copy link
Contributor

I'm going to work on this

@alroniks alroniks added blocked Active participation around the pull request or issue required. Consensus is not reached. and removed state/being-discussed labels Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Active participation around the pull request or issue required. Consensus is not reached. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.
Projects
None yet
Development

No branches or pull requests

8 participants