foundation / foundation-yetinauts

3 stars 0 forks source link

Foundation structure and code standard #18

Open ncoden opened 8 years ago

ncoden commented 8 years ago

Namespace

Currently, Foundation use a namespace in Js (Foundation.*), but not in HTML (data attributes, classes) or Sass (variables, functions, placeholders, mixins).

Because of HTML attributes or classes (though selectors) and Sass elements are all working in a global namespace, declaring Foundation-related names without a Foundation prefix can (and often) lead to naming collisions. These errors are incorrigibles for the user.

About HTML data attributes: https://github.com/zurb/yetinauts/pull/13. About Sass elements (related to naming convention): https://github.com/zurb/foundation-sites/pull/8693#discussion_r82352078

For HTML data attributes and classes, we can: (source)

For Sass elements, they can't be declared dynamically, so we can't make the prefix customizable. We only can use the zf- prefix everywhere in v7.

Components properties

Currently, Foundation doesn't define properties in a particular order. According to CSS-Tricks, properties are often sorted alphabetically or grouped by types.

Properties sorting chart

Like said here, I prefer ordering CSS properties by categories instead of alphabetically because it is easier to:

These categories are:

You can see the full properties sorting I'm using here, and also several other standards.

Components selectors

Inside Foundation elements, a lot of properties are currently defined by targeting <tags>, [attributes], .linked.class or .nested .classes. These selectors can lead in practice to several CSS problems, which can not be easily solvable:

The BEM methodology is a popular way to split components selectors into blocks (i.e. .house), elements (i.e. .house__window) and modifiers (i.e. house--blue). It allows to apply properties on and inside a component with always 1 level of specificity, avoiding the problems above.

A BEM methodology perfectly applied would lead to remove all !important, which would become unnecessary.

_Note: There is a lot of naming conventions that are based on BEM, block__element--modifier is only one of them_.

See also: https://www.sitepoint.com/bem-smacss-advice-from-developers/

Components organization

Currently, our whole Sass is quite horizontally organized around components. Almost every rules are declared (and repeated) within them, and sometimes it is whole pieces of codes that are duplicated. Except for breakpoint, we do not really use @mixins or %placeholders across components.

The ITCSS and SMACSS conventions help to avoid these code duplications by encouraging to declare and use base components inherited by objects and components, like in an Oriented Object approach.

ITCSS description

See also: https://github.com/zurb/foundation-sites/pull/8718#issuecomment-251820827

Naming convention

Our current naming conventions for classes and Sass elements is snipal-case. This convention make the component namespace indistinguishable from the class/function/variable(...) role. I personally almost never fully understand what does a Sass element by its name, and often have to look at this element code to know it.

For Sass elements, @kball also suggested we should easily understand their type by their name, like in other languages. I agree about it, but never seen an used naming convention following that principle, maybe because we already have .class, $variable and %placeholder.

However, we use in Sass the same concepts than classes that are blocks, elements, modifiers and states. We should adopt a naming convention that allows to distinguish them. The standard BEM naming convention was initially for classes, but can be extended to Sass elements.

Also, we use .modifier as modifier (for exemple .small, .medium, .expand...). Like the lack of namepace, it can cause naming collision, even between Foundation elements.

I purposed in https://github.com/zurb/foundation-sites/pull/8693#pullrequestreview-2996423 conventions that could follow almost all of these requirements:

Theme

We should take care about unwanted properties. It can be difficult (and hardly maintainable) to remove CSS properties from an HTML element, especially when they are applied with a high specificity.

Unfortunately, this is the case for almost all graphical properties declared within Foundation component. If they are often customizable with the settings, it is not possible for someone wanting a particular design to easily apply its own properties without having to care about what was defined previously. Graphical properties does not define the component behavior, they can be moved from the component core, so they should. We should allow to generate a theme-free version of Foundation.

This lead to reflexions about a way we could easily manage themes. All graphical properties are defined outside components, in a default theme included in Foundation that could be fully or partially loaded. The theme is also using is own theme settings file. This approach would help the Foundation users like the theme creators, that would not have to overwrite properties.

Settings

Currently, we have settings variables declared across components. These variables are overwritable and used to generated an unused scss/settings/_settings.scss that must be copied and customized by the user.

The approach allows to work on components with reachable settings, but there is in practice two minor problems:

With a theme, the settings files loading system would be expanded to:


What do you think ? Should I make this issue public in foundation-sites ?

brettsmason commented 8 years ago

Firstly, great writeup and thanks for putting this together. I'll try and answer each section one by one with my opinion.

Namespace

I don't really have an opinion on namespacing the JS - I'll leave that to the JS experts to talk about 😄 I'm not sure I see the need for a prefix in the Sass, just a better naming convention...

Component selectors

I agree that currently some of the Sass is a mess. Overriding styles and nesting, especially in the grid could use some work. Moving to a purely class selector approach would definitely help here.

Components organisation

I use SMACSS for my day to day jobs. I like it because its only a suggestion and is very flexible. More thought with our structure would definitely help.

Naming convention

I think I mentioned before I haven't used BEM before. I find it a bit off-putting, and it seems too strict and opinionated. Having said that, I definitely see the need for a stricter, more concise naming convention. I just think a bit more flexibility would be good. I'm keep to be shown why BEM is so good though 😄

I agree modifiers should of been .button-primary .button-small etc though.

Theme

I like the idea of this, but think I maybe need a working example to understand your suggestion a bit more. Separating style from function is a good plan though, just don't fully understand how we can accomplish that yet.

Settings

Again I think I probably need to see a working example to understand a bit more how it could work.

To summarise I think a naming convention is a great idea, but I'm not sold on BEM yet and think we need to be a little less opinionated in what we force the users to use. Also, when are you proposing all of this for? Version 7 or sooner than that?

ncoden commented 8 years ago

@brettsmason Thanks for your return. I hope I will answer to your questionings.

Namespace

I'm not sure I see the need for a prefix in the Sass, just a better naming convention...

Here an example of things declared by Foundation : $black, $white, $breakpoints, $code-color, @mixin fieldset, @mixin breakpoint, @mixin flex, @mixin button... When Foundation is imported, these totally generic terms can't be used elsewhere, nor redefined. All I can do is to avoid using these words in my projects. So yes, a prefix is necessary.

Naming convention

I'm keep to be shown why BEM is so good though

Let's play a little game. What does approximately the following class: .form-input ?

Theme

just don't fully understand how we can accomplish that yet.

See the end of https://smacss.com/book/type-module.

If we use a "always 1-level of specificity", the major difficulty is the CSS blocks orders. We can't simply generate all the components then the theme, because the block theme properties would overwrite the modifiers properties. So we have to include theme block, theme modifiers (etc...) @mixins inside the component block, modifiers (etc...).

If we declare all component elements and modifiers with a prefix (.component .component-modifier {...}), we can simply generate all the components rules, then all the theme rules.

Settings

Again I think I probably need to see a working example to understand a bit more how it could work.

  • default settings (some of them are defined within component files) my-project/vendor/foundation-sites/scss/components/house.scss
$house-width: 100% !default;
$house-color: #FFF !default; // Foundation Theme house color

my-project/vendor/another-great-theme/components/house.scss

$house-color: #000 !default; // Another Great Theme © house color
$house-width: 50%; // I prefer little houses
$house-color: #F00; // My house is painted with the blood of the developers that use style=
brettsmason commented 8 years ago

@ncoden Sorry I haven't had a change to properly reply again yet. I'll try to by the end of the week. A friend of mine pointed me to this: http://cssguidelin.es/ Interesting reading if you haven't seen it, and might prove useful on putting something together.

andycochran commented 8 years ago

I think this is great, @ncoden. My only concern is that we make sure it remains easy for folks to contribute to Foundation. It would be a shame for us to spend all our time policing PRs for adherence to standards that are too strict. But if BEM can make contributing easier—especially to newcomers—that would be amazing.

I do really like the SMACSS is-state classes we're using. BEM (or whatever we adopt) shouldn't conflict with that.

I agree with @brettsmason that whatever we choose shouldn't be too opinionated, or too prescriptive from a markup perspective. It should work for those who have less control over their markup or are creating their own custom/semantic classes.

Thanks for championing this effort.

ncoden commented 8 years ago

@andycochran With a BEM/SMACSS methodology, it would be harder for folks that don't know them to contribute, of course. However, each contribution would fit perfectly with the others and we shouldn't see all these naming/specificity/unwanted properties problems.

SMACSS uses BEM. BEM is a naming convention, and SMACSS a set of various conventions (naming, components types, declaration blocks...).

I think my proposals solve one problem you given (the need for custom/semantic classes). But what do you mean by "no control on markup".

ncoden commented 7 years ago

@kball Should we make this issue (and #17) public now ? And maybe the whole repo ?

kball commented 7 years ago

@ncoden lets definitely make this issue and #17 public... I would kind of like to maintain a private repo for the yetinauts because I think we may want to have info that is not publicly available. Can we make particular issues public? Or should we transcribe this over to the Foundation repo?

Or should we take a page out of the EmberJS book and create a new repo specifically for RFPs and other policy stuff?

ncoden commented 7 years ago

We can't make a particular issue public. For the other solutions: I don't know, you choose.

IamManchanda commented 7 years ago

I agree modifiers should of been .button-primary .button-small etc though.

@brettsmason .button--primary .button--small

IamManchanda commented 7 years ago

Or should we take a page out of the EmberJS book and create a new repo specifically for RFPs and other policy stuff?

@kball Yes just like https://github.com/laravel/internals/