Angular-cli: Components should create a `display:block` style by default

[hansl]

_From @SanderElias on September 7, 2018 11:19_

I'm submitting a...

[ X ] Feature request
[ X ] Documentation issue or request

Current behaviour

When you create a new component, by default it gets no setting for css display.

Expected behavior

it should have display:block by default.

What is the motivation/use case for changing the behavior?

I know this has been discussed before (#5960), and pops up in (#12340) too.
Still, I'm putitng this back on the roll. For the following reasons:

1. Current behaviour is confusing for new devs
2. Devs need display:block most of the time (for me, its 99+%)
3. The web doesn't break if we put this in place.
4. The expected impact of this "breaking" change is minimal
5. Saves a few bits from the app's payload
6. Less questions. no more, _why does my css..._.
7. One more thing less to remember when creating a component.

I want to elaborate on point 3 a bit. The same discussion has been held in by standarts committee, If you follow along that discussion you will see that the general consensus is that it is actually a good idea, but it can't be changed because that would break the web. Angular does not have that issue. Angular can change this, and the impact of the change would be minimal. probably something that has less impact as for example rxjs 5 to 6.

Others:
If we don't do this change, at least there should be more emphasis on this in the documentation.

_Copied from original issue: angular/angular#25856_

[hansl]

_From @Meligy on September 8, 2018 1:3_

This issue root is in how Angular keeps the component custom elements with their custom tag names in the rendered DOM, unlike other frameworks that omit these tags from output like React.

The design decision might not be bad overall, and is probably here to stay, but a problematic effect of this design is that these component elements get the display: inline; treatment by default. That's what browsers do when they see tags with no style, including built in user agent styles.

It's an instance where lack of action (no display setting) is equal to an action (setting display to inline).

Since it's not even inline-block, this often causes weird results when you try to use block styles to child elements in the template+styles of the component. The margins etc don't work as expected, and the Angular user has to track it all the way to understanding the above behaviour in order to understand and fix the problem.

This is where a documentation addition might help, also maybe using the Angular CLI default component schematic to add display: block; and/or a comment describing the behaviour, to default component styles.

[hansl]

_From @SanderElias on September 8, 2018 4:18_

@Meligy I'm aware of the history of this issue. I agree with the design choices. Still, Angular is in a position where we can change this browser default. The browsers can't do this, but Angular can.
Browsers behaviour as a default sounds good. However, in practice this thing more of a nuisance, as a benefit.
As in practice, you have a particular thing that is mandatory 99% of the time, and optional in 1%, it's a good idea to flip this around. No matter what, or how sound, the originating design choice is.

Angular is in a unique position, that can make the display:inline the option, and display:block the default.
Angular has also directives, aside from components. Those are here for the use-cases where you want a display:inline (or no real representation in the DOM at all.)

The cognitive load of Angular is already pretty high. Changing this would lower it a bit.

[hansl]

_From @trotyl on September 8, 2018 5:0_

@SanderElias I don't think this is about component, but only for custom element. A directive could also have element selector, like <mat-tree-node>, people might want that to be block as well. But a component may not use custom element, like <button mat-button>, adding a display: block; for <button> doesn't make any sense.

[hansl]

_From @chaosmonster on September 8, 2018 9:8_

I agree. I do this so many times that it feels a little bit cumbersome.
What I tried some time ago and it works under certain circumstances is to add a global css style

app-* {
  display: block;
}

That way I can override it where ever I need to.

Maybe something like this (using the prefixes defined in the angular.json) would be an option.
And I am aware that this wouldn't work with ViewEncapsulation.Native. It's more meant as a input to our creative thinking process.

[hansl]

_From @robwormald on September 9, 2018 21:53_

Thoughts: I agree with the stated problem (I forget this myself all the time!) but I'm not convinced that changing the default is a good idea. RE: cognitive overhead - my argument would be that any time that Angular diverges from the default browser behavior, that in itself increases the cognitive overhead (and load on docs team, etc).

It would likely break a number of apps as well - at least if we were to change something in the internals of Angular/Renderer, and (sorry) I'd argue again there's an increase in cognitive load here, because now something "invisible" (not in your component.css file) is changing behavior (and now you have to go look at the docs! How do I override it? etc )

If we want to avoid invisible / non-local problems (that is, I generally shouldn't have to look elsewhere to understand how a component works), I think that means that defaults like this have to exist in your app-component.css file.

We could use schematics or similar to augment the default css template, so ng g c foo would create a foo.component.css file like this out of the box:

:host {
  display: block;
}

One nice thing here is that many new developers have no idea that the :host selector is even a thing, so generating one for them teaches them something new for free.

One drawback here is that you then have to keep things "in sync" - if you change those default later, they won't affect already generated components.

We could provide a app/defaults.css file, that is added to the styleUrls of a new component:

:host {
  display: block;
}

```ts
@Component({
styleUrls: [
'app/defaults.css',
'./foo.component.css'
]
})

There's an argument to be made for CSS Variables (Custom Properties) here as well, because in a modern browser, rather than duplicating the `app.defaults.css` file per component, you would load it in index.html and do:
```css
:root {
  --default-component-display: block;
}

Then the generated foo.component.css would look like this:

:host {
  display: var(--default-component-display);
}

This one feels the best to me:

• it's explicit
• sticks close to the platform (so you can go read MDN about CSS Variables, rather than Angular documenting a slightly different custom API.

This requires CSS Custom Property support - the good news is this is every browser BUT IE nowadays: https://caniuse.com/#feat=css-variables

Since Angular does require a compiler pass, we could potentially make a best effort to "downlevel" those properties - at compile time, rewrite the dynamic var(--some-variable) to with the variables specified in the app.defaults.css file. This would not allow for dynamically setting properties at runtime, but would provide a middle-ground until we drop IE support.

CSS is forgiving, so if we were to generate something like this - on IE, the 2nd line doesn't parse (and so the 1st line is used) - on browsers that support CSS Variables, the --default-component-display is used (or the fallback block if not defined)

:host {
  display: block;
  display: var(--default-component-display, block);
}

This is broadly useful for theming as well, and potentially Material.

[hansl]

_From @SanderElias on September 10, 2018 4:27_

@robwormald I would even prefer this over changing the default. I don't agree that changing the default would raise the cognitive load. But let us not go there, as we agree on the solution path.

I prefer your solution because as you said, it exposes new devs to :host :{}. This does even a better job in discouraging them to not use a wrapping element in their components.
And in the same go, it shows them about the display:block, which most of them not even realize is a thing and might encourage them to learn some stuff about CSS.

Also exposes it them to CSS custom properties. which is another plus. It is about time that this will get some more exposure and broader pick-up.

I could live with the fall-back for IE, but I would like to see a setting to turn that off. Ie support should be a thing of the past. It is for me already (for a long time), but aside from that, I don't think we should carry this forward. Having a setting means that in a couple of years, we can switch the default of the setting, to not generate the IE fallback anymore(but still be available for devs that still need to support Ie).

[hansl]

_From @chaosmonster on September 10, 2018 6:17_

I really like the idea to solve this via schematics.

About resolving css variables at build time.
In a way I like the idea, but I would like to point out as @SanderElias said, that this should be a compile flag.
What I could imagine, and this should be a separate issue, is a preprocessor step within webpack, that resolves all css variables. This seems so generic, that I think it might already exist or at least should be out of scope of the angular compiler or what so ever.
Alternatively to resolving the variables it could also add css lines as shown in @robwormald last example.

[Meligy]

I like the idea of prefix-* selector in CSS.
It can be added along with comments explaining the issue and how to override the style using :host in the default Angular CLI style file, rather than adding another file to every component, and having to deal with whether the component has this default file or not, the order of file, specificity of selectors, etc.

If we want less cognitive overhead, and less magic, then not only we wouldn't add more default files, but I'd say also not to go for CSS variables for this specific case.

The CSS code itself would be commented like the CLI does with polyfills, and introduced in a feature (minor version) release. Later, it can be decided whether to uncomment it by default in a major version release, along with explanation in the release announcement of course.

[MarcusMorba]

To illustrate the issue which is a real problem especially with IE11:

[SanderElias]

@Meligy This change won't add a single file. It will add a bit of context to existing ones.
Also, most projects I have seen don't use a prefix, or use a prefix per feature, instead of a singe app-wide one.
By providing a sane default in the generated css for each new component you do reduce the cognitive overhead. Let me explain:

1. the display is set to 'block'. No need to remember that you need to do this.
2. if also shows how to use the :host selector. No need to look up how to do that again.
3. no more empty files, where you think, huh, what happened here and check history to figure out that it's just an empty file.
4. let me restate, its not an extra file, but default 'filling' for an existing empty one.

As this is in no way a breaking change, I don't think we need to wait for the next big release. As it happens on generation it only has influence on newly generated components, and exisitng ones remain untouched.
It is very much an convienent way to finally have a sane default in our components.

[clydin]

Regardless of the other points, this would be a breaking change as existing tooling solutions could very well assume that newly generated stylesheets are empty.

Something else to note is that a version of the css would be needed for each supported stylesheet preprocessor. Generate creates a stylesheet file specific to the default style setting.

Also, if this is indeed a sane default for all components, would this be better addressed at the Angular level itself?

[SanderElias]

@clydin By no breaking change, I meant it's not a breaking change for Angular or any app that is written with it.
It might be a breaking change for tooling, but I think assuming empty files is more a bug as a feature ;)

[dgp1130]

After discussion in the CLI meeting, I just wanted to add that Google has a web components doc which recommends setting :host { display: block; } as a best practice because custom elements use display: inline; by default (though I'm told Angular components are not custom elements).

There is also an interesting foot gun, in that if a user manually adds :host { display: block; }, it will break the hidden attribute. The user actually wants:

:host {
  display: block;
}

:host([hidden]) {
  display: none;
}

Somewhat unrelated, but another mistake a user might run into is using :host[hidden], which doesn't work due to custom element styles weirdness. If we do decide to make display: block the default, we should also include the extra style for hidden and possibly reference this doc as some justification.

[dgp1130]

I discussed this with @StephenFluin and @jelbourn, both of whom were on board with the idea of adding this to the CLI schematic as an option. Since we have a PR for this, I think it's reasonable to go forward with it. It's just a matter of nailing down the best way of implementing this.

Also, just to include this here: @jelbourn argued against my previous point about :host([hidden]), because the hidden attribute is generally an antipattern (precisely because of this problem) and it might be better not to tacitly nudge users towards using hidden over *ngIf and other tools provided by Angular.

[hiepxanh]

When this will be availabe for the current version? I see that It already merge to angular master branch

[dgp1130]

The PR was merged on Thursday so it just missed the last release, however it should be included in 9.1.0-next.3 which should be published later today.

Considering I don't think there's any more work to do here, I think we can close this issue. Just waiting for it to roll out.

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

<sub>_This action has been performed automatically by a bot._</sub>