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

[+] FO: Change structure for module assets override #5020

Merged
merged 1 commit into from Mar 14, 2016

Conversation

Projects
None yet
7 participants
@julienbourdeau
Contributor

julienbourdeau commented Feb 16, 2016

Theme can override module assets (CSS and JS only) by placing a file at a specific location. I slightly change the location.
Since the theme assets have moved under the assets/, it had to be changed. I thought moving everything module-related under modules would make more sense.

BEFORE

Type Original Override
JavaScript /modules/MODULE_NAME/js/file.js /themes/THEME_NAME/js/modules/MODULE_NAME/js/file.js
CSS /modules/MODULE_NAME/css/style.css /themes/THEME_NAME/css/modules/MODULE_NAME/css/style.css
JavaScript /modules/MODULE_NAME/view.tpl /themes/THEME_NAME/modules/MODULE_NAME/view.tpl

AFTER

Type Original Override
JavaScript /modules/MODULE_NAME/js/file.js /themes/THEME_NAME/modules/MODULE_NAME/js/file.js
CSS /modules/MODULE_NAME/css/style.css /themes/THEME_NAME/modules/MODULE_NAME/css/style.css
JavaScript /modules/MODULE_NAME/view.tpl /themes/THEME_NAME/modules/MODULE_NAME/view.tpl

Note: css/ and js/ have been removed between THEME_NAME/ and modules/

@kpodemski

This comment has been minimized.

Show comment
Hide comment
@kpodemski

kpodemski Feb 16, 2016

Contributor

👍

Contributor

kpodemski commented Feb 16, 2016

👍

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Feb 26, 2016

Contributor

👍
@kpodemski @julienbourdeau can you confirm this can't broke anything regarding the module links to stylesheets/javascripts ?

Contributor

mickaelandrieu commented Feb 26, 2016

👍
@kpodemski @julienbourdeau can you confirm this can't broke anything regarding the module links to stylesheets/javascripts ?

@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Mar 14, 2016

Contributor

Yeah it won't break anything in the modules, this commit only change the location of the theme override files.

Contributor

julienbourdeau commented Mar 14, 2016

Yeah it won't break anything in the modules, this commit only change the location of the theme override files.

@kpodemski

This comment has been minimized.

Show comment
Hide comment
@kpodemski

kpodemski Mar 14, 2016

Contributor

This change is theme related, theme is not backward compatible so... it is ok :)

Contributor

kpodemski commented Mar 14, 2016

This change is theme related, theme is not backward compatible so... it is ok :)

maximebiloe pushed a commit that referenced this pull request Mar 14, 2016

Maxime Biloé
Merge pull request #5020 from julienbourdeau/feat/module-media-path-o…
…verride

[+] FO: Change structure for module assets override

@maximebiloe maximebiloe merged commit eb8be4f into PrestaShop:develop Mar 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@julienbourdeau julienbourdeau deleted the julienbourdeau:feat/module-media-path-override branch Mar 22, 2016

@gabdara

This comment has been minimized.

Show comment
Hide comment
@gabdara

gabdara Jul 23, 2016

Contributor

The validator of modules tells us to put all js and css files in folder views, when overriding in theme should we place them also in folder views?

Example:

Original Override
/modules/MODULE_NAME/views/js/file.js /themes/THEME_NAME/modules/MODULE_NAME/views/js/file.js
/modules/MODULE_NAME/views/css/style.css /themes/THEME_NAME/modules/MODULE_NAME/views/css/style.css
/modules/MODULE_NAME/views/templates/front/moduledemo.tpl /themes/THEME_NAME/modules/MODULE_NAME/views/templates/front/moduledemo.tpl

Since the theme represents the View from MVC perspective, to me it seems a little redundant to have the views folder in the override location, but at the same time it's more clear where the original file is located this way.

Also because only js, css and tpl of module (which should be all located inside the views folder) can be overridden in the theme, maybe the views folder should be made mandatory for modules and the override from the theme looks only there to find the original file.

Example:

Original Override
/modules/MODULE_NAME/views/js/file.js /themes/THEME_NAME/modules/MODULE_NAME/js/file.js
/modules/MODULE_NAME/views/css/style.css /themes/THEME_NAME/modules/MODULE_NAME/css/style.css
/modules/MODULE_NAME/views/templates/front/moduledemo.tpl /themes/THEME_NAME/modules/MODULE_NAME/templates/front/moduledemo.tpl

This way I think it will be more obvious that in theme only files that are in the views folder of modules can be overridden and not php or other files that may be found in modules.

Contributor

gabdara commented Jul 23, 2016

The validator of modules tells us to put all js and css files in folder views, when overriding in theme should we place them also in folder views?

Example:

Original Override
/modules/MODULE_NAME/views/js/file.js /themes/THEME_NAME/modules/MODULE_NAME/views/js/file.js
/modules/MODULE_NAME/views/css/style.css /themes/THEME_NAME/modules/MODULE_NAME/views/css/style.css
/modules/MODULE_NAME/views/templates/front/moduledemo.tpl /themes/THEME_NAME/modules/MODULE_NAME/views/templates/front/moduledemo.tpl

Since the theme represents the View from MVC perspective, to me it seems a little redundant to have the views folder in the override location, but at the same time it's more clear where the original file is located this way.

Also because only js, css and tpl of module (which should be all located inside the views folder) can be overridden in the theme, maybe the views folder should be made mandatory for modules and the override from the theme looks only there to find the original file.

Example:

Original Override
/modules/MODULE_NAME/views/js/file.js /themes/THEME_NAME/modules/MODULE_NAME/js/file.js
/modules/MODULE_NAME/views/css/style.css /themes/THEME_NAME/modules/MODULE_NAME/css/style.css
/modules/MODULE_NAME/views/templates/front/moduledemo.tpl /themes/THEME_NAME/modules/MODULE_NAME/templates/front/moduledemo.tpl

This way I think it will be more obvious that in theme only files that are in the views folder of modules can be overridden and not php or other files that may be found in modules.

@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Aug 1, 2016

Contributor

@gabdara I'll have a look

Contributor

julienbourdeau commented Aug 1, 2016

@gabdara I'll have a look

@AndronCode

This comment has been minimized.

Show comment
Hide comment
@AndronCode

AndronCode Oct 13, 2016

In last line in table is it should be SMARTY template in Type column?

AndronCode commented Oct 13, 2016

In last line in table is it should be SMARTY template in Type column?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment