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

Avoid conflicts with existing stylesheets by adding specifity to css files. #36

Closed
wants to merge 9 commits into from
Closed

Conversation

teemualap
Copy link

This pr makes the provided css files more usable straight out of the box (since we can assume that a lot of developers may want to use grande.js as a component rather that a whole body element). Added specifity avoids conflicts with existing stylesheets and obliterates the need of !important flags in rules.

I also moved the body rules from editor.css to index.html since they likely exist for demo purposes only.

The css is an output from .less files with nested rules. I have this build process available in a separate branch if you want to have a look at it (build-process-ideas). There's more than that going on in that branch because I was thinking of a way to release grande.js as a minified and optimized bundle and alternatively give developers the chance to include the .less files directly into their grunt workflow (at least that is my use case). This effectively makes grande.js styles less opinionated and easier to customize.

@teemualap
Copy link
Author

If this modifies the files too much, I can make a smaller pr for just the problem parts. Though this works, it has some changes that are not necessary at the current state of this project.

@mduvall
Copy link
Owner

mduvall commented Sep 17, 2013

This does touch quite a bit, I'll play around with it tonight to make sure it doesn't reduce any regressions. I recently pushed a few CSS changes, could you rebase off of master? Thanks!

@teemualap
Copy link
Author

Ok, I tried this with an actual scenario and ran into an issue with .text-menu. It indeed is appended to the body instead of .g-body so this pr is based on faulty assumptions.

I will correct this and rebase off of the latest master.

@peleteiro
Copy link

IMHO, Grande.js should add a prefix to all classes, maybe .grande-*. I'm having a lot of problems with grande.js and bootstrap.

@mduvall
Copy link
Owner

mduvall commented Sep 18, 2013

@peleteiro Good point, filed an issue for this (#40) and will make sure it doesn't override any defaults from Bootstrap.

@teemualap
Copy link
Author

Yes, prefixing is a good idea at least for the wrapping classes. For example: .grande-menu and .grande-editor. Grande.js and bootstrap get along just fine with or without prefixing if we specify some rules a bit.

I will update this branch without prefixes for now.

@teemualap
Copy link
Author

This should be fine now.

@netoneko
Copy link
Contributor

netoneko commented Oct 5, 2013

I see that .text-menu is not prefixed for some reason?

@teemualap
Copy link
Author

Do you mean by .g-body? That is because .text-menu is appended to the body element had it .g-body class or not. Imo this is good because grande editor should not have to consume the whole body element.

Prefixing .text-menu differently is a good idea altogether, but does not relate to this issue.

@teemualap teemualap closed this Apr 14, 2014
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

Successfully merging this pull request may close these issues.

4 participants