Closed AdamJ closed 7 years ago
Suggest CSS Property Order:
Related property declarations should be grouped together following the order:
@mindreeper2420 I have some suggestions:
For General Formatting
For HTML Attribute order
<!-- The "type" defines the element's behavior. -->
<button type="" class=""></button>
For Class Names Naming convention How about using BEM Methodology? Instead of underscores we can use single hyphens
As you( @mindreeper2420 ) said using io as prefix is the great idea but what about planner/platform specific components? We should also come up with prefixes as that needs to be in guidelines
I have another point on lengthy HTMLs (horizontally). Can we limit the HTML line length for better readability ?
@SMahil Responses to your points below :)
Agree on General Formatting and HTML Attributes ordering. Those are definitely things that we can define that will also be easy to keep consistent.
I thought about the BEM Methodology (the PatternFly 5 Code Guidelines state that, but I commented them out when I forked them), but wasn't sure if we should go down that path right now. I'd be for it, just need to get the timing right.
This is what PatternFly 5 is recommending:
PatternFly follows the BEM methodology. It's a way to decouple CSS from HTML, and modularize class names so they can be extended.
Class name structure follows the {{pf-block}}__{{element}}--{{modifier}}
structure:
.pf-block__element--modifier {}
Example:
<div class="pf-panel pf-panel--collapsible">
<div class="pf-panel__title">
<h1>Heading</h1>
</div>
<div class="pf-panel__content">
<p>Lorem ipsum dolor sit amet.</p>
</div>
</div>
.pf-panel {} // Block
.pf-panel--collapsible {} // Modifier of block
.pf-panel__title {} // Element
.pf-panel__content {} // Element
.pf-panel__content--unpadded {} // Modifier of element
Why it’s better
As all of these are consumed by IO, I'd either stick with that, or go with an f8-
prefix for everything, as all of these areas are part of the Fabric8 organization. That would clearly separate what is specific to fabric8 and what is not. I think that breaking it down into Platform/Planner prefixes would decrease the extensibility of the overall environment.
I started working on a linter and set the max character line for Less files to 100 characters. We can do something similar for the HTML, but extend that to the ordering and formatting that you mentioned. A good piece of that would be specifically written in our code guidelines document.
Ready to close or still in review?
@dgutride @dlabrecq any thoughts on the Code Guidelines that we've written up?
Does Patternfly 5 have a code guidelines file available for reference? I'm wondering if we can point to that instead or at least have a pointer to it as the gold standard?
Looking at the proposed CSS property grouping order and seems difficult to follow. I don't know if folks will get the ordering right each time? Unless everyone understands that 'content' comes before 'quotes' and 'word-spacing' comes before 'color'?
You might want to note why we're following that particular order? If we're are expected to place 'word-spacing' before 'color', it would be good to understand why that makes a difference. That is, why are the selector groupings not alphabetical?
I'm not a fan of 100 chars limits, especially for HTML files with long selector names? I find 120 to be a more reasonable number. If the Angular code is allowed to be greater than 100 chars, other files should be the same. The fabric8-ui linter is 120
@dlabrecq this was copied from the PatternFly 5 code guidelines here: https://github.com/patternfly/patternfly-css/blob/master/CODE-GUIDELINES.md, but with our additions/alterations. I added a reference to it at the bottom of the file, but I can include one at the top as well.
On the CSS grouping order, PatternFly 5 will be following the Field CSS Manual, which this guideline has just taken the pieces out and put them into one list. The Field Manual does break them out in an easier to read format, so we can just refer to that.
Regarding the 100 character limit, that is coming from the Less Hinter that we will be using: lesshint. The default there is 100, so that is what I put for our limit as well. That can be changed, though I'd like to have a limit that isn't too out there.
The Field CSS manual shows appears to be alphabetical within groups -- color is before any other property. Just wanted to be clear about the proposed order because your suggested groupings were not alphabetical.
120 chars is not unreasonable for HTML.
PR for updating the Code Guidelines with information on Field CSS Manual, structure and HTML character limit: https://github.com/fabric8io/fabric8-ux/pull/508.
With the latest PR merged, I'm going to close this issue. We can open another if updates are needed, once we start to implement the guidelines across the platform.
This issue tracks the progress of the creation of the CodeGuidelines.md file that will govern the HTML and CSS for fabric8io/fabric8-ui and fabric8io/fabric8-planner.
The file can be found here: CODEGUIDELINES.md, or can be viewed on the fabric8-ux site.