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

extract bootstrap classes into theme? #582

Open
johndavid400 opened this issue Aug 12, 2014 · 3 comments
Open

extract bootstrap classes into theme? #582

johndavid400 opened this issue Aug 12, 2014 · 3 comments
Milestone

Comments

@johndavid400
Copy link

Hey Ryan,

First off, Forem is awesome! I have been using it on my site for a couple of years:
https://prototyperobotics.com/forums

I am in the process of upgrading to Rails 4 so I updated to the rails 4 branch of forem. I like the new theme engine, but I use Foundation instead of Bootstrap and was wondering if you could clear up a question or two that I had.

  1. what is the reasoning behind having bootstrap classes embedded in the views?

I ask because it seems that this makes Forem somewhat dependent on using bootstrap, even though you don't get the bootstrap goodness unless you use the bootstrap theme.

  1. I was wondering how you would feel about making the bootstrap classes (ie. btn, col-md-12 etc..) that are used in some of the views more generic, and then using sass to extend them from within the theme?

I did this when reworking your bootstrap theme for foundation and it seems to work well... though I feel that refactoring some css classes in the views might be necessary to make it easier for theming.

I would be happy to submit a pull-request with these changes for you to check out, but wanted to get your opinions before putting too much time into it. Here is an example of re-assigning the bootstrap classes with foundation styles in the foundation theme I am working on:

https://github.com/johndavid400/forem-foundation/blob/cleanup/app/assets/stylesheets/forem-foundation.css.scss

I was thinking of making the classes forem-based, like "forem-button" and "forem-button-red", and then extending those with the appropriate classes from the preferred css framework within the theme.

BTW, I am good friends with Josh Adams and met you at Spree Conf in New York a few years ago.

Cheers,
JD

@radar
Copy link
Collaborator

radar commented Aug 19, 2014

Hey @johndavid400, I remember you from SpreeConf :)

I agree. Forem should definitely have its own classes and not rely on Bootstrap. I think that this is definitely the way forward. I do not have any spare time right now to fix this within Forem, but I do have enough time to review a PR. If you want to go ahead and patch this up, that'd be great.

The reason I did that in the first place is mainly due to laziness (I don't know enough about CSS, clearly) and therefore I wanted to be "opinionated" about what CSS framework that was chosen. Therefore I stuck Bootstrap everywhere with the idea that people could just customize the Bootstrap theme. Obviously this isn't going to work if someone wants to use something else.

Thanks!

@frankie-loves-jesus
Copy link

@johndavid400 if you're going to be doing this - ideally all UI-related thingies should be moved to a separate Rails engine. I see a lot of people like doing their own UIs from scratch using their own favorite libs, so it makes little sense for them having to include stuff like Bootstrap, Select2 etc.

I was wondering how you would feel about making the bootstrap classes (ie. btn, col-md-12 etc..)

I disagree that Forem's views should be gridded up though. I mean I love grids, but only when it comes to graphic design and architecture. Why some people see the need to transfer them programatically onto their HTML and CSS to achieve things that regular CSS will do just fine, is beyond me. If it is alignment you're after, http://hashgrid.com/ might be able to help.

@radar
Copy link
Collaborator

radar commented Jul 13, 2015

the community will address this as part of the work on the new gem release (discussion in #666)

@radar radar added this to the v1.0 milestone Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants