search

luyadev / luya-module-cms

The LUYA CMS module provides a full functional CMS for adding contents based on blocks.
https://luya.io
MIT License
33 stars 46 forks source link

LangSwitcher should keep query string in URL (or at least have a such option) #220

Open Antikon opened 5 years ago

Antikon commented 5 years ago

One of the typical scenarios on page is to update some model. In this case an URL looks like http://host/lang/module/controller/update?id=5.

Switching the language should keep the same form but change the labels and so on.

At present the widget drops a query string, so the URL becomes http://host/lang2/module/controller/update. It causes Bad Request since the id is missed.

LUYA Core Version: 1.0.20 LUYA CMS Version: 2.1.0

nadar commented 5 years ago

yes indeed. adding query params should and must be possible. for quick fix: add an url rule, better for seo anyhow :-)

would you like to send a PR?

Antikon commented 5 years ago

Sure, but a little bit later

nadar commented 5 years ago

Would be nice. otherwise i will pick the issue but not within next few days. If you need assistance, let me know. The check for exisiting params should be done here i assume:

https://github.com/luyadev/luya-module-cms/blob/8639b26d336e9536fb77c36de197527eb91b615c/src/widgets/LangSwitcher.php#L199-L208

But we have to compare the get parameters against the current url - as get params will also be resolved when using url rules

Antikon commented 5 years ago

I've made some investigations. The LangSwitcher widget itself can add additional params to URL (see line 229). https://github.com/luyadev/luya-module-cms/blob/9ce31a834c05d7b14cb2e54f4b8f0bb544ad70aa/src/widgets/LangSwitcher.php#L214-L231

The URL rule in my case comes from ModuleReflection class https://github.com/luyadev/luya/blob/0860ac787cc5e37aa966446952e0f524ff3d3edd/core/base/ModuleReflection.php#L133-L180

My question is what is 'args' and 'originalArgs' and how they should be populated from parseRequest method (line 156)?

If we use $route = $this->request->resolve() instead, then the 'path' parameter from CatchAllUrlRule will be added also.

nadar commented 4 years ago

Does that help?