Another patch of the #694382: Remove cosmetic cruft from module's css files series.
Here we deal with book module CSS trying to remove any superfluous declaration. Only styles that makes the UI work are left (in this case in the page that let's you reorder a book).
More patch on the same line
#865084: Clean up block module css styles
#749530: Clean up aggregator css styles
Original code
/* $Id: book.css,v 1.12 2009/08/24 03:11:34 webchick Exp $ */
.book-navigation .menu {
border-top: 1px solid #888;
padding: 1em 0 0 3em; /* LTR */
}
.book-navigation .page-links {
border-top: 1px solid #888;
border-bottom: 1px solid #888;
text-align: center;
padding: 0.5em;
}
.book-navigation .page-previous {
text-align: left;
width: 42%;
display: block;
float: left; /* LTR */
}
.book-navigation .page-up {
margin: 0 5%;
width: 4%;
display: block;
float: left; /* LTR */
}
.book-navigation .page-next {
text-align: right;
width: 42%;
display: block;
float: right;
}
#book-outline {
min-width: 56em;
}
.book-outline-form .form-item {
margin-top: 0;
margin-bottom: 0;
}
html.js #edit-book-pick-book {
display: none;
}
.form-item-book-bid .description {
clear: both;
}
#book-admin-edit select {
margin-right: 24px;
}
#book-admin-edit select.progress-disabled {
margin-right: 0;
}
#book-admin-edit tr.ajax-new-content {
background-color: #ffd;
}
#book-admin-edit .form-item {
float: left;
}
After the patch
html.js #edit-book-pick-book {
display: none;
}
#book-admin-edit tr.ajax-new-content {
background-color: #ffd;
}
#book-admin-edit .form-item {
float: left;
}
To complete the patch I removed the book.module rtl css file (there aren't any LTR declaration left) and transferred some declaration (first 5 of the original file) to Bartik and Garland (along with the RTL counterpart).
This means you won't see any difference but there will be even less styles to override when developing themes.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | book_stark.jpg | 88.52 KB | dodorama |
| #5 | book2.patch | 4.82 KB | dodorama |
| book.patch | 5.23 KB | dodorama |
Comments
Comment #1
jacineIt's counter productive to remove code from the module CSS file and add it to multiple core themes. It just ends up making duplication and more code to maintain, so even though the default UI is design is less than desirable, it is what it is, and still belongs in the book.css file. Trimming down the CSS to the bare, but efficient minimum to maintain the default UI is what we need to do.
Comment #2
dodorama commentedThe scope of this patches is to remove from module's css files every attempt to style elements that aren't strictly necessary for the correct functioning of the UI. At the same time I'm cleaning up the CSS and in fact not every declaration removed is added back in core themes . Still some elements need styling but this doesn't necessarily mean code duplication, (i.e Bartik could style the book navigation differently from Garland like it does with many other elements).
The point is that this extra code in core makes it difficult to style some elements and themers spend most of the time trying to override them.
Comment #3
jacineWell... Bartik and Garland could easily provide their own CSS for the book module by providing their own copy of book.css and custom themes can decide to remove it entirely. I don't think that process difficult. There's also the matter of loading the book module's CSS on every page, when it may or may not be enabled at all. This patch assumes that every custom theme will think the default implementation is bad, and will need to provide code for it - even if it's a matter of copying and pasting from a core theme. IMO that is a larger burden that having to override what's there in the first place.
I'm all for cleaning up CSS code wherever possible, I just don't think removing the structural aspects (which are really UI design changes when you look at it in Stark or any other custom theme) has anything to do with cleaning up actual CSS code at this stage in the game. I'd like to see colors and borders removed, etc. and the cleanup done at the source.
Hopefully that makes sense. If you disagree, or still want to go about changing the default UI, it's probably best to change the title and component of this issue.
Comment #4
dodorama commentedI agree with you and I believe we're saying the same thing. The point is where's the line, what belongs to core and what belongs to themes. My idea is that only declarations that make the UI works (along with javascript most of the time) should be included in core css. All the rest belongs to themes.
This can be inefficient sometimes but at least is a really clear separation. Otherwise it could be really hard to decide what to keep in core and what to leave to themes (is book navigation styling a structural aspect?).
Does this clarify my position and makes clear the scope of this patches?
Comment #5
dodorama commentedBy the way here's a revised patch with a softer approach. I just removed the book navigation styles (but still they're back in Bartik and Garland) and left admin stuff.
Original code
After patch
Still I don't get if there's a reason for zeroing .form-item margins, adding a min-width to the book outline form and a margin-right to the select form in the book reorder page. I guess these 3 declarations could go, too but I need assistance.
Attached a screenshot for Stark
Comment #6
Jeff Burnz commentedThis is the UI for book module and removing it is a far bigger discussion than we're meant to be having here - e.g. remove the border color but not the border (in a manner of speaking).
Comment #7
jacineSorry guys, this one is going to have to wait until D8 unfortunately.
Comment #8
rootworkThe CSS for the D8 book module has been refactored and seems quite minimal now. It doesn't zero anything out and it also uses relative widths (percentages instead of pixels or ems). And it only uses classes, so there will be fewer specificity issues in overriding it.
I think this is probably fixed.