e107inc / e107

e107 Bootstrap CMS (Content Management System) v2 with PHP, MySQL, HTML5, jQuery and Twitter Bootstrap. Issue Discussion Room: https://gitter.im/e107inc/e107
https://e107.org
GNU General Public License v3.0
321 stars 213 forks source link

Use universal class theme name for frontend theme #4141

Closed Jimmi08 closed 4 years ago

Jimmi08 commented 4 years ago

Motivation

With the 2.3 version frontend theme class is presented. Now class name is the theme name (bootstrap3_class etc) .

After making some themes for the new version this could be confusing for users not familiar with this. Before you just copied and renamed the theme folder, now you can't forget to rename this class too, otherwise, your site will not work.

Proposed Solution

To use some universal class name for this - frontend_theme or something like this.

Additional Context

I tested it after gitter discussion as CaMeRon suggested, no visible issues.

Jimmi08 commented 4 years ago

Example for bootstrap3 theme:

class bootstrap3_theme
{

    /**
     * @param string $caption
     * @param string $text
     * @param string $id : id of the current render
     * @param array $info : current style and other menu data. 
     */ 
    function tablestyle($caption, $text, $id='', $info=array())
    {

each theme uses different class:

    class bootstrap4_theme
    {
CaMer0n commented 4 years ago

@Jimmi08 This is done. Please give it a try and leave any comments here. Thank you.

Jimmi08 commented 4 years ago

@CaMer0n there is always Please place all theme code inside the theme class. With bootstrap3 theme too. image

CaMer0n commented 4 years ago

@Jimmi08 Message me on Gitter? I am unable to reproduce this.

Jimmi08 commented 4 years ago

@CaMer0n Noticed weird class with news category <a class="_category" Reason is:

    function sc_newscategory($parm=null)
    {
        $category_name = e107::getParser()->toHTML($this->news_item['category_name'], FALSE ,'defs');
        $category = array('id' => $this->news_item['category_id'], 'name' => $this->news_item['category_sef'] );
        $categoryClass = varset($GLOBALS['NEWS_CSSMODE'],'');
        return "<a class='".$categoryClass."_category' style='".(isset($this->param['catlink']) ? $this->param['catlink'] : "#")."' href='".e107::getUrl()->create('news/list/category', $category)."'>".$category_name."</a>";
    }

NEWS_CSSMODE is not used anymore. Maybe next value for your legacy list?

CaMer0n commented 4 years ago

Thanks @Jimmi08 , it's unrelated and I believe this class could be removed completely, since the template container should allow one to still target the a tag.