Prestashop: Classic theme should promote code quality and best practices - upgrade and rework

Created on 8 Jul 2019  路  23Comments  路  Source: PrestaShop/PrestaShop

Is your feature request related to a problem? Please describe.
I'm always frustrated when I open .scss files coming from the classic theme.
I see there is plenty of efforts that are being put on the backend side of Prestashop, but I'm not feeling like this is the same for the front end part.

Yes I know that there has been some pretty good changes like the venue of Webpack, ES6, on the tooling part.

But when I'm looking on files like checkout.scss, I feel a bit like it's not a really enjoyable environment to work on. Don't take it like an offence, but I think this file for example is full of bad practices:

  • components/checkout.scss should be something like pages/checkout.scss because its everything but a component.
  • 39 ID selectors in one file, free CSS
  • nesting is sometimes so deep that we can find a Balrog in there
...
      &.taxes {
        td {
          text-align: left;
          .value {
            padding-top: 0;
          }
        }
      }
    }
  }
  • so many combined selectors like its the Guinness book of CSS specificity
body#checkout section.checkout-step.-reachable.-complete h1 .step-edit {
  • empty blocks, because hey, the cart is empty
.cart-empty {
  .cart-summary {
  }
}
  • styling attached to .js- classes and style that should really go inside the Atos module
.cart-empty {
  .cart-summary {
  }
}
.js-payment-binary {
  display: none;
  .accept-cgv {
    display: none;
  }
  &.disabled {
    opacity: 0.6;
    cursor: not-allowed;

    // A div overlay to prevent using clicking on stuff !
    // not even working on my Chrome 75.0.3770.100 (Official Build) (64-bit)
    &::before {
      content: "";
      position: absolute;
      top: 0;
      right: 0;
      bottom: 0;
      left: 0;
      pointer-events: none;
    }
    .accept-cgv {
      display: block;
    }
  }
}

And I could continue for a day like this.

All this to tell you how I feel when seeing this .scss code, knowing this had been rewritten from scratch for 1.7.

Describe the solution you'd like
I think like the classic theme should really promote good coding standards, best practices, and would really benefit from a complete cleanup.
For now, I don't even think like Prestashop themes are a good environment for junior developers to learn html and CSS.

I was positively surprised when the starter theme came out, as it was in my sense a really good move toward code quality. But it's been discontinued.. too bad.

Front-end Improvement Modules & Themes TBS

Most helpful comment

First of all, I fully agree with you @tlhuillier. The classic theme has not been receiving enough love -- in part due to the fact that we have had no fulltime front developers in the team for a while now. Please don't hesitate to send pull requests! Just try and make them small so that they can be merged quickly.

@SharakPL I wish it was like that, but it's not. This has been explained several times in Github discussions and is also one of the reasons PrestaShop/classic-rocket is a thing.

The development team in charge of the original 1.7 decided, back in the day, to develop new themes ("classic" for the FO and "new theme" for the BO) using the not-yet-released-at-the-time bootstrap 4. As they tell me, they thought that Bs4 would be final in time for final release of 1.7, and started working with the alphas... with turned out to be a _very bad call_.

Bs4 Alpha 6 was released in January 2017, _after_ initial release of PS 1.7.0 (Nov 2016). Worse, that alpha switched the grid model to flexbox, breaking everything. Since most themes are based on classic (as @mickaelandrieu explained), upgrading Bs4 in classic would break existing themes. So we are stuck.

In PrestaShop 8, we'll be able to switch to a new theme. In the meantime, there's classic-rocket which is basically an improved classic theme.

All 23 comments

@tlhuillier please join to https://github.com/PrestaShop/classic-rocket project and participate in its development :)

Hey @PrestaShark
Your project seems like an interesting initiative, I'll give it a shot.

But still, the Classic theme (being shipped with Prestashop) should be the quality standard on top of which others could rely to take example.

Hi @tlhuillier,

Thanks for your report.
ping @PrestaShop/prestashop-core-developers what do you think?

Thanks!

How could we disagree with such a statement 馃槃 obviously you're right @tlhuillier .

It's true that there is so much work on the backend side that we sometimes forget about the theme 馃挃 . Also since a lot of people use their own theme, they are more eager to report (and speak about) bugs about the backend logic than the theme.

There's just one thing that will be a blocker: retro-compatibility. Even if there are some useless CSS or IDs around, we cannot remove them because modules/themes rely on them. We'll have to wait for next major PS version (1.8.0.0) to be able to do such a breaking change.

Glad to ear that.

1.7 themes not being retro compatible with 1.6, I think those things should have been removed totally since modules needed some updates to work on 1.7 and themes needed to be totally rewritten anyway.

One step toward dropping this compatibility code could be to move everything related to retro-compatibility inside a shame.scss ? or retro-compatibility.scss to be able to easily spot them. And the day Prestashop is ready to make another round of breaking changes, simply delete this file.

@tlhuillier The reason for bad quality of classic theme is simple and quite understandable: it's just bussiness. If classic were good enough less people would consider buying a theme from prestashop. That's fair considering it's free. The only thing I can't understand is why would anyone use alpha or even beta packages in production. It's been 1.5 years since stable Bootstrap 4 was released. It's 4.3.1 already and classic theme still uses 4.0.0-alpha.5. That's not even the newest of alphas! 馃槚

@SharakPL
Good points.

The classic theme being free doesn't mean it shouldn't be exemplary.
When a usual merchant buys a theme on Addons, it's not the code that he will look at, only the FO demo and screenshots anyway.
But for the sake of the community, if we wan't more quality themes (and thus happier merchants), we need to have a solid Classic theme.

Hi,

The only thing I can't understand is why would anyone use alpha or even beta packages in production. It's been 1.5 years since stable Bootstrap 4 was released. It's 4.3.1 already and classic theme still uses 4.0.0-alpha.5. That's not even the newest of alphas! confounded

Most of the themes on Addons are already based on the Classic theme, so updating bootstrap 4 to the latest (we all want to do that, believe me) will break every theme on the market place first, then will break every website once they will upgrade their shop.

Simply said: the core team can't do that before the next major release but you can rely on the community theme classic-rocket which is mostly up to date regarding bootstrap :)

First of all, I fully agree with you @tlhuillier. The classic theme has not been receiving enough love -- in part due to the fact that we have had no fulltime front developers in the team for a while now. Please don't hesitate to send pull requests! Just try and make them small so that they can be merged quickly.

@SharakPL I wish it was like that, but it's not. This has been explained several times in Github discussions and is also one of the reasons PrestaShop/classic-rocket is a thing.

The development team in charge of the original 1.7 decided, back in the day, to develop new themes ("classic" for the FO and "new theme" for the BO) using the not-yet-released-at-the-time bootstrap 4. As they tell me, they thought that Bs4 would be final in time for final release of 1.7, and started working with the alphas... with turned out to be a _very bad call_.

Bs4 Alpha 6 was released in January 2017, _after_ initial release of PS 1.7.0 (Nov 2016). Worse, that alpha switched the grid model to flexbox, breaking everything. Since most themes are based on classic (as @mickaelandrieu explained), upgrading Bs4 in classic would break existing themes. So we are stuck.

In PrestaShop 8, we'll be able to switch to a new theme. In the meantime, there's classic-rocket which is basically an improved classic theme.

@eternoendless
Thanks for the clarifications, you made it very clear.

@eternoendless lack of flexbox is the most negative part of it 馃槩 I hope it will be proper way in 1.8 with Vue instead jquery for product, cart and order related components.

What do you think of this issue @NeOMakinG ?

@colinegin Full answer has already been given by @eternoendless and @mickaelandrieu . There is nothing we can do to address these issues on Classic Theme because doing so would break compatibility with all existing themes ... and anger the whole community doing themes for prestashop.

I think we should put this issue in PrestaShop 8 (prestashop next) kanban.

Thanks @matks , i've put it in the dedicated kanban.

I have an idea ...
You should stop working on classic v1 and move to classic v2 or another name and build it from scratch with most newest tec....
And More thing ... Best mobile responsive..
Easy to edite...
Easy css improvement...
Make it 馃挴% compatible with rtl ... And so on

I have an idea ...
You should stop working on classic v1 and move to classic v2 or another name and build it from scratch with most newest tec....

This would benefit all people that would use v2 as a starting point, but it would leave behind all the people using v1. We cant do that. People trust us to update this v1 theme and we will not let them down 馃槈 . It's the trust between us, the maintainers, and the community 鉂わ笍

Simply said: the core team can't do that before the next major release _but_ you can rely on the community theme _classic-rocket_ which is mostly up to date regarding bootstrap :)

https://github.com/PrestaShop/classic-rocket is somehow a v2 馃槈although developed by the community, not us

Maybe you guys should mention this stuff in the docs as it is very confusing for devs to get into it. Ive spend a couple of hours figuring out the contradictory docs and reasons classic theme is still on bootstrap alpha, fixing the build process to run with latest Mac OS node versions etc. Maybe just a link and small text to classic-rocket would help the devs new to this topic. Sometimes this is crucial as a dev recommends to the employer wich software they should use and when he sees the deprecated stuff and the docs he wont consider prestashop..

Maybe you guys should mention this stuff in the docs as it is very confusing for devs to get into it. Ive spend a couple of hours figuring out the contradictory docs and reasons classic theme is still on bootstrap alpha, fixing the build process to run with latest Mac OS node versions etc. Maybe just a link and small text to classic-rocket would help the devs new to this topic. Sometimes this is crucial as a dev recommends to the employer wich software they should use and when he sees the deprecated stuff and the docs he wont consider prestashop..

Nice idea, I will do it 馃槃thanks !

I think this discussion is complete so I close this issue.

Actually I reopen 馃槄 this issue goes into the Kanban https://github.com/PrestaShop/PrestaShop/projects/6

@matks I don't know what to do with this issue, we can't rework it as everyone wants, because it would introduce so much BC breaks, this shouldn't go to the kanban tbh...

The only quick solution would be to develop a parallel theme, which is probably what we don't want, the next one is to wait the major version in order to refacto this theme.

wdyt @PrestaShop/prestashop-maintainers ?

I'm the one of few maintainers of community-theme for 1.6 (it's in PrestaShop organization), give me green light and I'll progress with similar project on 1.7

@matks I don't know what to do with this issue, we can't rework it as everyone wants, because it would introduce so much BC breaks

The right solution is to rework the Classic Theme for PS 8.0.0 . So it's a slow long-term solution.

As for a quick solution, we encourage people to create their own Theme base such as Classic Rocket but these must be community-only.

There are many issues with starting a new Theme for PS 1.7

  • it means we have 2 themes to maintain, every change in one Theme must be done in the 2nd (2x review, 2x QA, 2x time)
  • we must document properly how to use both: 2x docs, tutos, examples
  • if we are consistent, we must make sure native modules are compatible with both : 2x work for native FO modules

Then, when a developer that sells Themes and Modules on addons.prestashop.com he'll wonder what target he should hit. The modern new theme but maybe then the audience he can reach is very small ? Or the old bug-packed theme with hundred of stores already using it ? So we fragment the Theme market for PS 1.7 .

@kpodemski this is the reason why we never started, since Starter Theme, a new FO Theme project. It is actually very expensive for the maintainer team. If you think there is a need and you feel like addressing it, you can start a community-theme for 1.7 馃槈 you are free of course. But if it is not in the project main scope, no QA or developer time will be assigned to it 馃槥 .

The maintainance issues can be solved with more developer time but the Theme market fragmentation is bad for community of sellers on Addons 馃槥

Was this page helpful?
0 / 5 - 0 ratings