vuestorefront / vue-storefront

Alokai is a Frontend as a Service solution that simplifies composable commerce. It connects all the technologies needed to build and deploy fast & scalable ecommerce frontends. It guides merchants to deliver exceptional customer experiences quickly and easily.
https://www.alokai.com
MIT License
10.66k stars 2.09k forks source link

Default Template 2.0 - feedback #1768

Closed pkarw closed 5 years ago

pkarw commented 6 years ago

We're currently working on Default theme 2.0 - the design process is on its way. I think this is the right time to gather some feedback regarding code architecture/structure as well. This ticket is for that - please feel free to include Your voice in the discussion

What is the motivation/use case for changing the behavior or adding a new feature?

Default Theme in its current state is getting a little bit messy. After being one year in the business we also have some ideas how to make things simpler. On the design layer, on the other hand, we do have a lot of ideas on how to improve User Experience.

Tech feedback

Here we go with the ideas on how to improve the architecture of the themes

Too granular mixins

  1. I think we should avoid a "two-liner" mixins like https://github.com/DivanteLtd/vue-storefront/blob/master/core/modules/cart/features/isMicrocartOpen.ts. These small mixins are very difficult to even discover by developers, the comment + brackets generate more lines than the core logic (which is just .. .Vuex call). On the other hand, it makes the compile process longer (IO operation to import the module etc.). Moreover, it makes theme development more difficult (You have to create a lot of these mixins to meet good practices). I think we should keep things simple the core components were easy to understand and use for theme developers. If we're to keep the features in module/features we should rather have kind of bundled mixins (with a set of features) - but this makes core components redundant.

I'm a fan of making things simpler and we should stay with core components + features that really do something (other than just calling Veux actions). Modules are a great idea but in this forms make a little good to the project. They make it more difficult to understand.

Current theme was thought of single/central theme without modularization as the scope of the theme is from my perspective too thin to divide it to modules (and the effects are these too granular mixins in my opinion).

We should aim more at WordPress/Shopify approach NOT Magento approach regarding themes.

Too complicated directory structure

  1. We do have sth like:
    • /theme/components/core/
    • /theme/components/theme/
    • /theme/components/core/blocks
    • /theme/components/theme/blocks

We should simplify it - we don't need blocks folder and probably don't need theme as well. I think that the theme should be divided into sections corresponding to Pages and Components (maybe modules) + one "Common" folder - that's all.

filrak commented 6 years ago

Hey! Good idea, I'd love to hear some feedback before coding.

Regarding 'Too granular mixins'

Too complicated directory structure Totally agree. This architecture became a litle bit too complicated. Core components should be (and will be) a part of modules (and only at the root level of module_name/components like everything else). It's much better to split them into behavior they represent than imaginary blocks. Core pages should stay since they are not belonging to modules.

Now I want to write a few features that new them will have and why we need them

jorkvist commented 6 years ago

Hi, just my 2cents i think before you starting on version 2, the first version should have much better quality else you will be in a state where v2 is a excuse to not fix stuff in v1. Else the community will have a setback of maybe +1y(?) before the v2 is ready and during that time you there will be a lot of stuff in v1 that's would be good to have solved for people using that in production.

pkarw commented 6 years ago

@jorkvist we're all committed to fix v1 as well :)

pkarw commented 6 years ago

@filrak I see that we have totally oposite opinion on the mixins. I really dislike this very granular approach - which from my perspective - makes theme development (I mean from scratch, not using the existing components) a nightmare. You can hardly find whose mixins interfere with others, how the dependencies look like. It's good only in theory but it would be great to hear some opinion from @szafran89 @Igloczek @qiqqq who used to work with the themes

The good thing is that we agree in almost all other points :)

filrak commented 6 years ago

@pkarw each mixin should be standalone, work without any dependencys and not interfere with any other ;) If some functionality will rely on the other then they should be merged

Please remember that we will still have this core components. Not ALL the component parts needs to be splitted into features and not all of them can be but generally it's a good approach to allow people compose their components in the way they want without forcing to use our complex ones if they need to.

Even tho most of them will still use core compoennts I still think that this ablity to just use features you need is something VS is lacking right now. Also it's much more transparent when you see exact features instead of just abstract components.

And again it's much easier to test and refactor/update code splitted in this way. Grouping code into standalone features prevents this features from relying on each other.

@jorkvist now we are doing huge amount of work on improving v1 and v2 will be more or less backward-compatible so it's not like we are leaving everyone behind with the old codebase ;)

DavidRouyer commented 6 years ago

Can we rewrite components which have dependencies at the same time? For example, https://github.com/DivanteLtd/vue-storefront/blob/master/core/components/ProductGallery.js should be generic and not dependent of any dependency

HRR1337 commented 6 years ago

What about using TailwindCSS with Purgecss and 'Above the fold CSS'? As fast as possible..

mercs600 commented 6 years ago

Yo guys! So I agree with @pkarw default theme it's getting messy. Generally we should start thinking more about functional/dumb components. But what we have now...

@pkarw

Too granular mixins

These small mixins are very difficult to even discover by developers, the comment + brackets generate more lines than the core logic (which is just .. .Vuex call).

I know what you feel, but I agree absolutely with @filrak. From my perspective I would like to choose what I need use in my components than maintenance part of code which I don't need. Comments are not problem because are not occur in build code.

Now we have small mixins where you can find just vuex call - right, but we never know what we will need to add to logic of these mixins later. We should keep some level of abstraction from Vuex. If we build some serious and simple documentation (https://vuejs.org/v2/api/ should be good example for us :heart_eyes:) there will be no problems to find accurate mixin.

Now we give user core components build from feature mixins and this is great because he may take whole cart module with all needed logic. But again some of users may want to use only specific mixin and this is the point for us. Trust us :wink:

Too complicated directory structure

We should simplify it - we don't need blocks folder and probably don't need theme as well.

I could not agree more with you. I think user may not know what is core component and what is theme component. I think blocks was nice idea, but we lost the track what it should be exactly :smile:

@DavidRouyer

Can we rewrite components which have dependencies at the same time? For example, https://github.com/DivanteLtd/vue-storefront/blob/master/core/components/ProductGallery.js should be generic and not dependent of any dependency

Totally agree. New default theme or base set of components should be consist of only dumb / functional components. We shouldn't keep the "data" logic in components like this. They should works only based on props.

@HRR1337

What about using TailwindCSS with Purgecss and 'Above the fold CSS'? As fast as possible..

I'm not advocating for rely on some CSS frameworks in application such like VS. Stupid example with flexbox grid with some wrong padding calculation on mobile devices. We had a lot of problems to refactore current theme to flexbox grid 2 :cold_sweat: because a lot of components was using flexboxgrid classes and what worse some part of our code overwrite default code of flexboxgrid - nightmare.

Few of my thoughts

1) We should pull out themes from our code base. Theme should be external package which user can maintain. Users should be able to update VS core without some hardcore merge.

2) Core logic components - we should give user all needed logic to create specific part of shop like cart, product page, checkout etc. I think we are on the good way with api features and core components like cart.

3) If we want to give possibility to simple and quickly customize we have to create some set of base theme components:

  1. In my opinion (perhaps only in my opinion :grin: ) we should avoid scoped styles. They are troublesome.

  2. We should start thinking about functional components - Theme components should rely only props. They shouldn't get data directly from vuex or API. Data and logic should be provide page wrappers, in our case this will be product page / category page / checkout page. Dumb components will be helpful to improve performance, readability, scalability and testing.

to be continued...

DavidRouyer commented 6 years ago

We should think about the scope of the theme too. Let's talk about pixel density for example. Today, we only support a pixel density of 1 (take the images for example, we don't use the srcset or the element to support multiple pixel density...), but do we want to support them further and making the theme overcomplicated?