Closed (fixed)
Project:
Bootstrap
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
1 May 2013 at 17:44 UTC
Updated:
27 May 2013 at 20:21 UTC
Jump to comment: Most recent file
We should discuss the differences between the LESS subtheme files, and the basetheme style.css. There are some good fixes in the subtheme LESS, and also a good bit of the basetheme style.css that gets overwritten. Perhaps the basetheme style.css should more closely resemble a compiled version of the subtheme LESS to start with, or perhaps move some of the oft-overridden bits of the basetheme style.css into the subtheme theme.css
- Possible to rename basetheme "style.css" to "theme.css" (or vice-versa) for consistency?
- Discuss some of the fixes and their reasoning in the fixes.less
- Combine/compile new basetheme CSS based on subtheme LESS
Also, does anyone else have issues getting the subtheme LESS to compile properly with the LESS module? I get some responsive scaffolding-size errors when using the module, but not when using a local compiler. See: http://drupal.org/node/1331144#comment-7364480
Comments
Comment #1
markhalliwellI completely agree. TBH, I forgot I had the "fixes.less" file in the sub-theme as it was originally a github repo before I made it a patch for this project, my bad.
Ultimately, yes I think that we should port everything over to the less version (for simplicity and ease of management) and also compile it to the css folder when using the CDN method. We should also only include the compiled CSS when using the CDN method as adding it when using the source method would just duplicate code.
I think we should rename both to
overrides.less/csssince they override both drupal and bootstrap styling that isn't necessary or conflict with each other.re: less issues, that should probably be a separate issue. I read the related issue you posted it seems to possibly be an issue with the php library. TBH, I haven't done a lot of testing with Bootstrap and LESS yet since I have a local compiler. However there is a lot I'd like to do with LESS bootstrap integration. Particularly solving things like dynamic variables, bootswatch support and library support (using custom functions to return the import path). These are all things that will come to pass, but will just take time.
Comment #2
markhalliwellRenamed the 'theme.css/less' to 'styles.css/less' in the starter-kit (made more sense given other themes). Renamed, rewrote and combined all provided CSS into one 'overrides.less' file. Only this file should be compiled to the root css folder (minified). That compiled overrides file will now only be loaded if using the CDN, so removed it from the .info files. Removed pull-left from the search form and added the float in the overrides. Added 'clearfix' to the block template, needed it. Updated READMEs docs.
Comment #4
markhalliwell#2: bootstrap-improve-lesscss-consistency.-1984354-2.patch queued for re-testing.
Comment #6
markhalliwellHmm, let's try this?
Comment #8
markhalliwellOk, I'm going to try this semi-manual patch (instead of using drush_iq).
Comment #9
markhalliwellAdded styling for individual checkboxes and radios with labels after, per #1649392: Checkbox or radio inputs without titles don't render
Comment #11
markhalliwellFor the love of.... I have no idea why I'm having issues automating this issue's patches. But here's another manual
git format-patch, sigh.Comment #12
markhalliwellAny chance you've had a moment to review this?
Comment #13
andregriffin commentedThe subtheme CSS was not included in the info file. I think this was in error, since the subtheme stylesheet should be added, regardless of method 1 or 2 being used? I also think the "style.css" is more often used, rather than "styles.css", so I changed the file names and docs to prevent some inadvertent typos/keep things consistent. Let me know what you think.
Otherwise, I think this is all great progress.
Comment #14
andregriffin commentedWhoops, lost the unversioned files in #13.
Comment #16
andregriffin commentedComment #17
andregriffin commentedUgh
Comment #18
markhalliwellGreat catch and good suggestion. Went ahead and committed: 6cc387b. I suspect these patches are failing due to the numerous renaming/deletions that are taking place. Let's open smaller issues (edit: as they come up) now that the majority of this has been taken care of.
Comment #19.0
(not verified) commentedClarification