Follow-up to #2398455: Clean up "book" component in Bartik
Problem/Motivation
The book module CSS is not inline with our standards. The selectors need to be rethought as components and the properties could probably do with a clean up.
Proposed resolution
To remove these problems, we should follow the best practices guidelines outlined on this drupal.org documentation page: http://drupal.org/node/1887918#best-practices
Remaining tasks
Discuss the naming of the components
Implement the component structure
Clean up CSS properties if required
User interface changes
None. The design objects that style the Drupal module's functionality should be look roughly the same after as they do now.
API changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | refactor-book-module-css-2405445-11.patch | 4.27 KB | crazyrohila |
| #8 | interdiff-6-8.txt | 2.86 KB | crazyrohila |
| #8 | refactor-book-module-css-2405445-8.patch | 4.27 KB | crazyrohila |
| #7 | Child_page___s66e87dd2a810783_s3_simplytest_me.png | 793.59 KB | lewisnyman |
| #6 | interdiff-2-6.txt | 3.43 KB | crazyrohila |
Comments
Comment #1
lewisnymanComment #2
crazyrohila commentedwe have following components:
book.theme.css
- navigation
- pager
book.admin.css
- js dependent stylei
Created two component files in css folder, and not sure about *.admin.css. attached initial patch.
Comment #3
crazyrohila commentedThese classes
.book-pagerare in classy templates. This is applicable to module only. So should we remove all of this css from module and put them in classy theme?Comment #4
lewisnyman@crazyrohila Thanks for the patch, we actually have different file naming standards with modules than we do with themes, see the coding standards docs. I think we are better off grouping them under book.theme.css for now. It also looks possible to reduce the component to just one, we could probably clean up the template a little and remove unused classes.
Is this styling used? I don't see a menu class but I do see the book-pager class, maybe we can just use that?
Maybe we can remove the book-navigation class and just have book pager?
Comment #5
lewisnymanI think this might be a good opportunity to introduce a reuseable class to achieve this called js-hidden.
Comment #6
crazyrohila commented@lewis thanks for review.
1.

.menuis being used for child book page list for a page.2.
.region-content ulandul.menuare overriding style when using.book-pageronly3. There was class in
system.module.csscalled.js-hideI have used that one here.Comment #7
lewisnymanNice! I tested the patch to make sure the styling still applies and it does. We can tidy up the old classes though, I noticed that they were still printing now they aren't being used.
We've deleted this file but we also need to remove reference to this file so it's not being loaded, we can remove the admin library in book.libraries.yml and also remove reference to that library:
We need to update this comment to: " the js-hide class hides this button when Javascript is enabled"
Comment #8
crazyrohila commented1. I thought they can be useful as fallback as if I am not aware of all places where they have been used. Removed Now.
2. Removed
admin.cssfrom libraries too.Comment #9
crazyrohila commentedComment #10
lewisnymanWe can still remove classes before we hit release candidate 1.
Ah almost! We need a full stop at the end of this comment :( Then we are done!
Comment #11
crazyrohila commented@lewis thanks for pointing.
Updated.
Comment #12
lewisnymanGreat, thanks!
Comment #13
alexpottNice tidy up. Markup changes are not frozen in beta. Committed d246439 and pushed to 8.0.x. Thanks!