Themes loader #1913

zixxus · 2019-04-04T19:29:57Z

What are you changing/introducing

At first, this require:
https://github.com/zixxus/luya/tree/theme
https://github.com/zixxus/luya-env-dev/tree/themes
https://github.com/zixxus/luya-module-cms/tree/themes

Add Theme component
Add global variable @theme
Add folder /themes with XML file where we configure the template
Remove resources & views folder - now its be replaced

What is the reason for changing/introducing

I decided that many people prefer to manage their templates in the same way as it is in wordpress or prestashop. In the coming days, the template management module will appear in the admin panel.

In the future, he also proposes to add child themes, which will help the developer sell one template to many clients.
Currently the default theme is / themes / blank. We can change them through the sql code:

SELECT * FROM admin_configWHEREname = 'active_theme' limit 1;

QA

Q	A
Is bugfix?	no
New feature?	yes
Breaks BC?	no
Tests pass?	no

boehsermoe · 2019-04-05T15:04:46Z

Nice feature! I had thought of something like that too. At first please revert your filemode changes like .gitignore 100644 → 100755 for more info see here
After that it is easier too review. Thank you!

zixxus · 2019-04-06T12:19:30Z

Nice feature! I had thought of something like that too. At first please revert your filemode changes like .gitignore 100644 → 100755 for more info see here
After that it is easier too review. Thank you!

Thanks :) I change file mode, but I don't know i should create new pull request?

nadar · 2019-04-06T16:39:58Z

I will do a review tomorrow, this looks promising for me! i have some Inputs regarding naming, and i think we need an object for the theme itself, isntead of an array with keys.

is there a preview of the themes management module somewhere?

nadar

@zixxus i did a short review now. I think we can not merge this PR, because:

luya core must work without database, its just the MVC extended basic application.
the theme should be stored in an object not an array with keys

I made some comments on the given situations, see above.

Summary:

yes themes loading is a good idea, what about the yii framework way of this? pro/cons? https://www.yiiframework.com/doc/guide/2.0/en/output-theming
yes, maybe the themes component should be the main place for communication inside the application, but logic which theme is loaded etc. should be done in the CMS module via bootstraping.

nadar · 2019-04-08T08:43:22Z

core/base/Boot.php

@@ -246,6 +246,10 @@ public function applicationConsole()
 ],
 ]);
 $this->app = new ConsoleApplication($mergedConfig);
+
+ $this->theme = Yii::$app->themes->getTheme();


setting alias should be done in the bootstrap section /base/BaseBootstrap

nadar · 2019-04-08T08:43:47Z

core/base/Module.php

+ /**
+ * @var string The default file of layout
+ */
+ public $layout = 'main';


layouts must be defined in the controller, i don't think we can introduce this variable here.

nadar · 2019-04-08T08:44:29Z

core/base/Module.php

+ /**
+ * @var string The default folder of theme load
+ */
+ public $theme;


The theme should be configured via the component, so i think the path for the layout should be extract from the themes component.

nadar · 2019-04-08T08:45:54Z

core/base/Module.php

 * @return string
 * @see \yii\base\Module::getLayoutPath()
 */
 public function getLayoutPath()
 {
+ $this->theme = Yii::$app->themes->getTheme();


The layout file must be set in the controller, as mentioned above. Also not sure about error handling, cause there should be the question:

what kind of errors can a theme have?

nadar · 2019-04-08T08:47:08Z

core/components/Themes.php

@@ -0,0 +1,188 @@
+<?php


In general, yes i agree we could added a themes component to luya which will be the main channel to communicate whether a theme exists or not, and which one is active. etc. So maybe we could keep this component, but improve it and make it the main channel to communicate between controller, views and application.

nadar · 2019-04-08T08:47:24Z

core/components/Themes.php

+ public function setup()
+ {
+
+ $this->config['active'] = $this->getActive();


instead of an array we need an object.

nadar · 2019-04-08T08:49:03Z

core/components/Themes.php

+ */
+ public function getActive()
+ {
+ try {


This won't work for other DBMS system therefore we would need active query, but its not allowed to make database requests in LUYA core as its mvc only and no database is needed. So the CMS must inject this information to the themes component in a bootstrap section for example.

nadar · 2019-04-08T08:49:49Z

tests/core/components/ThemesTest.php

@@ -0,0 +1,52 @@
+<?php


not sure how usefull those tests are 😄

nadar · 2019-04-08T08:54:45Z

In addition i also would like to say that we maybe need an issue where we can discuss the GOALS of the themes option and how we can get there. What are specifications of a theme? how are they stored (xml? database?) how to install and load, etc.

zixxus · 2019-04-08T19:10:00Z

I review your comments and firstly I propose:

Rename all class and functions to Layouts (and similar to layouts)
Class Layouts {

init() - for initialize all script
getLayouts() - get all avaliable layouts (return all avaliable layouts, scan theme dir and load layout data from xml file)
getLayout() - get one layout by name (with layout data like name, descr, author etc. loaded from
getActive() - get current "on" layout (only name)
getDir() - get Directory where layouts is stored.

}

Rewrite all arrays to objects.

2.1) Rewrite sql to Model.

2.2)

In general, yes i agree we could added a themes component to luya which will be the main channel to communicate whether a theme exists or not, and which one is active. etc. So maybe we could keep this component, but improve it and make it the main channel to communicate between controller, views and application.

I think it's a good idea, I will work on it.

Move code whose initialize layouts to CMS module and load it in controller (luya\cms\frontend\base\Controller).

In core i'll leave only Layouts component and also load this component.

In the administrator module I leave only Layouts managment wherein we can switch themes. Maybe I add function to create / load child themes but at a later time :)

Are you think I can split theme managment for two modules or it will be better when all code put into luya-module-cms/src/admin? In this I thinking about page in whose we can managment themes. This page will be placed for example in: https://demo.luya.io/en/admin#!/template/admin~2Flayouts~2Findex

Remove test class
Clear variables ( some variables can be place in component / controller).
Add theme uploader (.zip) and autoinstaller it.
Create docs for theme like what xml files should look like and etc.

yes themes loading is a good idea, what about the yii framework way of this? pro/cons?

Often I write themes and modules for wordpress, my clients like when page is simple, If he need to change theme or theme variables its be required in admin panel not in code. Also some clients buy themes and scipts and it need installer like "Theme install" button. I think it be more usefull than YII array with theme managment and more programmers will start using luya because it will be more readable.

What are you think about this? :)

boehsermoe

Also some clients buy themes and scipts and it need installer like "Theme install" button.

Of course it is easier for a client but at first it should be easy for the developers to install and configure a theme. This could be a later feature when there is something like a "marketplace" for luya themes.

And also need a api for themes.

What is with extending or overriding exists themes? This does not have to be implemented at the beginning but should be take into consideration.

As @nadar says in general we need a issue to collect and discuss features/goals/ideas.

boehsermoe · 2019-04-09T05:52:03Z

core/components/Themes.php

+
+
+ if (file_exists($path . '/theme.xml')) {
+ $xml = simplexml_load_string(file_get_contents($path . '/theme.xml'));


Theme infos should be stored in a json because theme config has no complex structure. Maybe it could also stored in the composer.json in the "extra" section?

nadar · 2019-04-09T07:12:41Z

@zixxus @boehsermoe i made an issue, so we can discuss there and maybe close this PR:

#1916

zixxus · 2019-04-09T07:15:06Z

@zixxus @boehsermoe i made an issue, so we can discuss there any maybe close this PR:

#1916

OK :)

zixxus added 4 commits April 4, 2019 19:40

Themes loader component

4e14ff8

Add theme test

68d0187

Add theme to boot

b3c699d

repair instalation

2e75969

File mode restore

323797d

nadar reviewed Apr 8, 2019

View reviewed changes

boehsermoe reviewed Apr 9, 2019

View reviewed changes

nadar closed this Jun 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Themes loader #1913

Themes loader #1913

zixxus commented Apr 4, 2019

boehsermoe commented Apr 5, 2019

zixxus commented Apr 6, 2019

nadar commented Apr 6, 2019

nadar left a comment

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar Apr 8, 2019

nadar commented Apr 8, 2019

zixxus commented Apr 8, 2019

boehsermoe left a comment •

edited

Loading

boehsermoe Apr 9, 2019

nadar commented Apr 9, 2019 •

edited

Loading

zixxus commented Apr 9, 2019



		if (file_exists($path . '/theme.xml')) {
		$xml = simplexml_load_string(file_get_contents($path . '/theme.xml'));

Themes loader #1913

Themes loader #1913

Conversation

zixxus commented Apr 4, 2019

What are you changing/introducing

What is the reason for changing/introducing

QA

boehsermoe commented Apr 5, 2019

zixxus commented Apr 6, 2019

nadar commented Apr 6, 2019

nadar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadar commented Apr 8, 2019

zixxus commented Apr 8, 2019

boehsermoe left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadar commented Apr 9, 2019 • edited Loading

zixxus commented Apr 9, 2019

boehsermoe left a comment •

edited

Loading

nadar commented Apr 9, 2019 •

edited

Loading