Skip to content
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

Deprecate _skeleton.scss in favor Bootstrap in the frontend #2758

Closed
jacobherrington opened this issue Jun 1, 2018 · 9 comments
Closed

Deprecate _skeleton.scss in favor Bootstrap in the frontend #2758

jacobherrington opened this issue Jun 1, 2018 · 9 comments
Labels
changelog:solidus_frontend Changes to the solidus_frontend gem

Comments

@jacobherrington
Copy link
Contributor

The erb templates in frontend are currently reliant on Skeleton V1.2 (stored locally), which uses fixed width columns.

I'm working on a project that will deprecate Skeleton in favor of Bootstrap for a few these templates. I'm happy to change all of the templates and PR back to frontend if there aren't any objections.

@jacobherrington
Copy link
Contributor Author

Looking at #580, I'm not sure if this would be a welcome change. Open to feedback.

At the least, I would prefer to replace _skeleton.css with a grid system that isn't fixed width.

@tvdeyen
Copy link
Member

tvdeyen commented Jun 7, 2018

@jacobherrington I say give it a shoot.

@jacobherrington
Copy link
Contributor Author

Based on my own anecdotal experience, I think Solidus frontend is very important for newcomers to Solidus and potential adopters. I also think the current frontend is a hurdle for newcomers because it is a bit dated.

I would love to build a new frontend with a powerful framework like React or Vue, but In the interest of keeping our footprint small...

Short-term
We will replace _skeleton.css with an extremely light, more-modern CSS grid system. I feel like that is a good incremental quality of life change that will help new Solidus developers in building their own shop frontend.

Long-term
In light of our previous conversation @tvdeyen, I will look in to building a modern starter frontend for Soldius that will function as a drop-in replacement for the current frontend.

@tvdeyen
Copy link
Member

tvdeyen commented Jun 7, 2018 via email

@kennyadsl
Copy link
Member

Hey there, sorry for being late in answering and thanks for your thoughts and work on this topic.

I have a quite strong opinion against introducing another grid along with the basic frontend template provided. The reason is that what we are providing is just a template that the majority of Solidus users just uses as reference to start building their own frontend. Using one framework or another is a choice that developers should make based on the design they need to implement, so in terms of grid framework in my opinion Skeleton and Boostrap are equivalent since they'll be replaced. We'd just replace unsemantic class names with other unsemantic class names that developer will need to change. Skeleton is here just because this choice was made a lot of time ago when the idea of changing/extending Solidus (Spree at that time) frontend instead of totally replacing it was something real. Honestly I can't see any advantage just changing grid framework at this point.

My proposal:

What I'd say is a change useful to the project is replacing unsemantic class names for grids (or any other) with something that is rich of meaning and could both self-document code and provide an html structure that could be taken as is when a custom frontends are implemented.

For example, instead of:

<div id="content" class="columns <%= !content_for?(:sidebar) ? "sixteen" : "twelve omega" %>" data-hook>
  <!-- ... -->
</div>

Having something like:

<div id="content" class="<%= !content_for?(:sidebar) ? "content-without-sidebar" : "content-with-sidebar" %>" data-hook>
  <!-- ... -->
</div>

At this point we could move all grid sizing information into the scss, separating markup from its style. In this way Solidus html will not depend anymore from a specific framework, it's self-documented without any specific knowledge and can also be easily copy-pasted into developers projects without too much changes.

To move grid sizing information into the scss as mentioned above, Boostrap 4 already provides such scss mixins.

I look forward to read your opinion on this and thanks again.

@jacobherrington
Copy link
Contributor Author

Hey @kennyadsl, thanks for your feedback.

A few things came to mind as I read your response. I'd like make it clear that I am a newcomer to Solidus. My observations come without the context of your history with Spree or prior knowledge of Solidus.

You're right that Solidus shouldn't enforce a frontend framework, which is part of the reason we chose to take only the grid classes from Bootstrap. This allows us to make use of a well-tested grid system, without forcing developers to adopt the entire Bootstrap framework. Furthermore, It's no more difficult to remove Bootstrap grid than it is to remove the Skeleton grid. Regardless, if you expect developers to create their own frontend anyway is there any harm in bringing the example frontend up to date?

Personally, I can see a lot of value from adopting a modern, well-documented flex based CSS grid. As a newcomer I was very confused by the presence of a fixed-width grid system from 2011. Do you think it is valuable to give newcomers examples that are easier to work?

Rather than rebuilding the entire frontend, #2767 specifically addresses the presence of an outdated, fixed-width CSS grid that might be a stumbling block for newcomers. Essentially this is a pragmatic solution to get Skeleton out of the repository.

Thanks again for you feedback and it's great to hear your opinion on the future of Solidus frontend.

@kennyadsl
Copy link
Member

Still not convinced! 🙂

What's the scenario when you, as a developer building your frontend taking Solidus views as a template, could benefit from this grid change?

First of all I want to specify that I totally agree that Bootstrap grid is better than the Skeleton grid.
My concern is more with changing what we have in favor of a non-optimal solution: we'd risk to break frontend views of people that used the Solidus frontend without overriding them (using Deface) with another solution that we could want to change in a bit.
We'd force people to revisit their deface overrides and style (or reintroduce skeleton on their store) in favor of a solution we already know it's not the ultimate. I'd take the risk of forcing users to take an action to improve their store, but I'd like to be sure its the right one, which in my opinion is using semantic class names and moving style things to css.

Anyway I don't want to be the only one that blocks this so, if other core team members have a different opinion, feel free to go head with this approach.

@tvdeyen
Copy link
Member

tvdeyen commented Jun 12, 2018 via email

@jacobherrington
Copy link
Contributor Author

Sounds good, we will look into it when we have time. 👍

@jacobherrington jacobherrington added UI changelog:solidus_frontend Changes to the solidus_frontend gem labels Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_frontend Changes to the solidus_frontend gem
Projects
None yet
Development

No branches or pull requests

3 participants