Closed haroldangenent closed 9 years ago
@luukdv will talk ofzo. On monday
OOCSS will be implemented for 1.1.0.
By the way: This means a complete rewrite, taking into account the main principles of OOCSS:
Please update https://github.com/trendwerk/trendpress/wiki/Mixins after fixing this.
Soms remarks on _functions.scss
:
@section
and @subsection
seems needless, let's simplify our comments and use SassDoc for re-usable lego's, functions and mixins;I'm really enthusiastic about this, thanks for your work @gewoonderrik and @luukdv! Did a small review, you'll find a better overview on the current situation below. A lot of things are questions, I hope one of you could answer these.
directory/
, except when this is impossible.display: block
with margin-bottom
and display: block
with float: left
? Maybe even a structure like this (I'm not sure about this):block()
block-float()
(extends block()
)block-margin()
(extends block()
)border-radius
mixin can be removed, it's not useful anymore (since autoprefixer)block()
mixin is too non-description. I'm not sure what it does and it doesn't even display: block
gradient()
, it's not used and this is not intended as a library. Fezcat might be interested :-)parenthesis()
and some don't. Should all have parenthesis.placeholder()
in favor of style simplification. Current use is exactly the same on both occasions: @include placeholder($text-color, $text-light);
. This should be default styling on the element level.remove-list-styles
to list-style-none
.sprite()
(the images as well probably): It is not used and this is not intended as a library. They are site-specific things and should be implemented on a site-specific level. Of course, that implementation method should be efficient.$white-space: nowrap
declaration from the button
mixin. It's unused.main
as default for $type
, since main is never evaluated toutils/
. According to the guidelines used): The utils/ folder gathers all Sass tools and helpers used across the project. Every global variable, function, mixin and placeholder should be put in here.
(so only for Sass things, I'd recommend giving them their own file)
In a separate directory:
- _base.scss (also includes paragraphs)
- _headings.scss
- _italic.scss maybe remove this
- _lists.scss
- _quotes.scss
- _code.scss
- [x] Remove italic declarations (in case they are not necessary, has to be cross-browser tested)
footer
as a selector, instead of a class. This is also done on main
and there's only one footer by default. Maybe there should always be one footer each page.header
as a selector, instead of a class. This is also done on main
and there's only one header by default. Maybe there should always be one header each page..hentry
is the right class. This would inherit the same styling to new post types and you'd have to change this file afterwards. How about .type-post
?secondary
, should be wrapped in quotes._comment-respond.scss
, since it contains that class and only that class.small a
declaration - doesn't do anythingp:last-of-type
declaration.comments
with the same output (no chance on specifity problems).sitename
and .sitedescription
to .site-name
and .site-description
for readability_burger.scss
or rename .burger
to .navigation-icon
. I'd recommend the latter (maybe even .mobile-navigation-icon
, to keep things description and consistent).search-form-...
except for .glass
? Would it be possible to make things more consistent?_glass.scss
or rename .glass
to .search-icon
. I'd recommend the latter (maybe even .search-form-icon
, to keep things descriptive and consistent).alignleft
can be used for more things than images, right? Maybe this file should be split or something._wp-images.scss
, since we also have _wp-caption.scss
.wordpress/
foldercomponents/wordpress/
for all WordPress components we need.pages/
I'll start working on this as well. Maybe you guys can add to this list and keep it updated, so we have some kind of overview on the status. Big items or features can be placed as new issues.
I'd love to hear your thoughts.
We talked about this. Notes:
Separate structure and skin
We should do this.
Inclusion order
Alphabetically, except when it's impossible.
Mixins / functions
Will get their own directory. One mixin or function per file.
Button mixin
Split in several mixins. Mixins used once will be declared in the appropriate class.
Meta mixin
Will be removed.
Block mixin (proposal)
We should try this.
Block mixin (old one)
Rename it to something semantic.
Gradient
Move to Fezcat.
Parenthesis
Every mixins should have parenthesis.
Placeholder
Remove as proposed.
List-style-none
Yes.
Sprite mixin
Remove as proposed.
Helpers
Remove.
Typography
In a separate directory:
_base.scss
(also includes paragraphs)_headings.scss
_italic.scss
maybe remove this_lists.scss
_quotes.scss
_code.scss
Footer
Keep the class.
Header
Keep the class.
Hentry
Different class and file name.
Small a
Keep it.
p.last-of-type
Remove it.
Comments ol, ul
Put in parent. Remove ul
.
Gallery
Improve. Should be a separate issue.
.burger
Rename to .mobile-navigation-icon
.
.glass
Rename to .search-form-icon
or something. Also rename the file.
_wordpress-images.scss
Split if .alignleft
is used for more than just images.
New idea: create a wordpress/
folder for all WordPress components.
pages/
Remove.
The current block() mixin is too non-description. I'm not sure what it does and it doesn't even display: block
Based on some other frameworks and readings, I'd name it .panel
, .card
or .box
. Maybe .box
is the best one, since it's the most descriptive. The content is literally boxed in. I'll use that one, feel free to protest if you disagree.
Just refactored wordpress/
components. They include:
.wp-caption
.comment-respond
.comments
.gallery
_images.scss
(containing WYSIWYG image classes)WordPress components can be defined as such: Components which HTML is generated by either WordPress itself or it's WYSIWYG editor.
I have decided to drop #543 out of this issue and rename this one. The modular approach is practically finished and can be improved upon in time. Separation of skin and structure is a separate issue completely.
Agreed, as discussed. Excited about the merge.
@gewoonderrik has tested it. Removed assignment.