SuiteCRM icon indicating copy to clipboard operation
SuiteCRM copied to clipboard

Splits tpls into more meaningful sections --> increase customizability

Open HVStechnik opened this issue 5 years ago • 11 comments

As discussed in #7874 we prefer to look for custom template files each time a Smarty include function is executed. This is much clearer than looking for the custom tpl within the including template as proposed by #8028.

The fix #8053 allows to customize every included template. As a next stept, it makes a lot of sense to split the large tpl files into smaller pieces, so that developers can pick a specific subtemplate and customize just that subtemplate.

Description

This PR splits Smarty template files in more logical sub-templates and uses the Smarty include function to integrate the new sub-templates.

This PR splits the following files:

  • themes/SuiteP/include/DetailView/tab_panel_content.tpl
  • themes/SuiteP/include/EditView/tab_panel_content.tpl
  • themes/SuiteP/tpls/_headerModuleList.tpl

The changes are non-breaking, because we keep the names (and paths) of the three base templates unchanged.

Motivation and Context

It is just so much easier to customize a single line of tpl code here and there, if not everything is packed in a single file.

PS: The _headerModuleList.tpl (including the new sub-templates) is a mess. I do not really understand why it is necessary to have the same menu three times in the html code just to be responsive (one for desktop, one for tablet, one for mobile). I think there are better solutions to make the menu responsive without duplicating code. However, this PR does not touch this - it just seperates the original code into several tpl files.

How To Test This

This PR should not change the compiled templates.

The increase in lines results from the licence agreement, which is included in every new tpl file. In fact, we could reuse sub-templates in several locations, so that the actual amount of template code is reduced.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • [x] My code follows the code style of this project found here.
  • [x] My change requires a change to the documentation.
  • [x] I have read the How to Contribute guidelines.

HVStechnik avatar Oct 15 '19 15:10 HVStechnik

@HVStechnik It looks like the license headers are out of date on files. It also appears there are some filemode changes included here. Please can you tidy up and push your updates for us to re-review? Thanks for your contributions :D

willrennie avatar Oct 17 '19 14:10 willrennie

@willrennie Sure! Can you give me a hint, which filemode those template files should have? 775 as the entire theme directory? Thanks!

HVStechnik avatar Oct 17 '19 14:10 HVStechnik

I seem to remember there's a git option to ignore file mode altogether when commiting...

pgorod avatar Oct 17 '19 15:10 pgorod

@pgorod Yeah, should be:

git config core.fileMode false

and

git config  --global core.fileMode false

You should also be able to undo any file permissions changes you've already done with this:

git diff --summary | grep --color 'mode change 100755 => 100644' | cut -d' ' -f7- | xargs -d'\n' chmod +x
git diff --summary | grep --color 'mode change 100644 => 100755' | cut -d' ' -f7- | xargs -d'\n' chmod -x

Dillon-Brown avatar Oct 17 '19 15:10 Dillon-Brown

Codecov Report

Merging #8062 into hotfix-7.10.x will increase coverage by <.01%. The diff coverage is n/a.

@@                Coverage Diff                @@
##           hotfix-7.10.x    #8062      +/-   ##
=================================================
+ Coverage           10.5%    10.5%   +<.01%     
=================================================
  Files               3325     3325              
  Lines             243587   243593       +6     
=================================================
+ Hits               25583    25584       +1     
- Misses            218004   218009       +5

codecov-io avatar Oct 17 '19 16:10 codecov-io

Thanks @Dillon-Brown, @pgorod, and @willrennie! I hope that the new push does not include filemode changes any more...

HVStechnik avatar Oct 17 '19 17:10 HVStechnik

Assessed :+1:

As this is an enhancement could @mattlorimer or @samus-aran please take a look at this and merge if you're happy with the changes :smile:

Mac-Rae avatar Oct 24 '19 17:10 Mac-Rae

Updated to include #7879. @Dillon-Brown: Could you please review once again? Thanks!

HVStechnik avatar Nov 28 '19 12:11 HVStechnik

Codecov Report

Merging #8062 into hotfix-7.10.x will not change coverage. The diff coverage is n/a.

@@              Coverage Diff               @@
##           hotfix-7.10.x    #8062   +/-   ##
==============================================
  Coverage          10.70%   10.70%           
==============================================
  Files               3230     3230           
  Lines             241058   241058           
==============================================
  Hits               25798    25798           
  Misses            215260   215260           

codecov-commenter avatar Jul 05 '20 08:07 codecov-commenter

I've updated this PR to reflect recent changes in the master. @Dillon-Brown / @Mac-Rae: You reviewed this PR in the first place. I'm a bit disappointed that the merge into the codebase takes so long... The longer it takes, the more often I've to update the PR to account for other changes to the templates. Could you please review this once again? Thanks!

HVStechnik avatar Jul 05 '20 09:07 HVStechnik

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

SuiteBot avatar Aug 27 '20 13:08 SuiteBot