Bug: Generic input selector rule has side-effects

[lajoima1]

Describe the bug A clear and concise description of what the bug is. In scss/core/forms.scss line 242 there are generic input selectors being used without being specific to fundamental. This causes some unwanted side-effects for users.

SAP/fundamental-ngx#476

input[type=checkbox], // <-- causes unwanted side-effects
input[type=radio] { // <-- causes unwanted side-effects

Is this issue related to a specific component? Any input with type checkbox/radio

What version of the Fiori Fundamentals are you using? 1.4.1

What offering/product do you work on? Any pressing ship or release dates we should be aware of? The linked issue's owner is on SAP FSM

What front-end framework are you implementing in, e.g., Angular, React, etc.? Fundamental-NGX

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Smartphone (please complete the following information):

• Device: [e.g. iPhone6]
• OS: [e.g. iOS8.1]
• Browser [e.g. stock browser, safari]
• Version [e.g. 22]

Additional context Add any other context about the problem here.

[xak]

We include a light reset of core elements to match the expected look and feel. Can you provide an example of how this is causing a problem? Are you getting clashes with other libraries? Our intention is to provide a convenience meaning a class is not required for every input. When would you use a radio button in a Fiori look and feel next to one with a different look and feel?

[desaroxx]

@xak We were acquired by SAP last year and are now in the process of restyling our fleet of "already in production" Angular-based apps to comply with fundamental's look and feel.

As soon as we include fiori-fundamentals.css the "light reset of core elements" starts overriding our ALL of our input elements within an app.

We can not risk taking the "big bang" approach with including fiori-fundamentals.css and have a unknown number of input fields be affected.

We need to be able to migrate our apps gradually, meaning that we consciously migrate each input field individually.

What would make our life easier? All CSS Rules are prefixed with fd- prefixed. This we can control where fiori-fundamentals.css's styles get applied.

[droshev]

I think fiori-fundamentals should not set rules for elements that are not using fundamental classes. @saad-mo is there a way providing a solution for @desaroxx? It is blocking their adoption of fundamentals /implementation.

[bcullman]

@droshev: alternatively, fiori-fundamentals should allow for consumers to "opt-in" (or opt-out) for non-classed element style

[eboyer]

@bcullman @droshev ~ Do you guys have any thoughts on an "opt-out" approach if we were to leave the code as it is now AND provide some opt-in classes? Removing the base styles would be a breaking change for a major version, so we'll have to be sensitive of that if we end up doing it.

[ChristianKienle]

@eboyer @bcullman @droshev @desaroxx what about integrating the styles in your app like this:

$fd-icons-path : './../node_modules/fiori-fundamentals/scss/icons/';
$fd-scss-font-path : './../node_modules/fiori-fundamentals/scss/fonts/';

@import './../node_modules/fiori-fundamentals/scss/core/root.scss';
@import './../node_modules/fiori-fundamentals/scss/settings';

.fiori {
  /* stylelint-disable-next-line declaration-no-important */
  font-size: 14px !important;
}

.fiori[data-fiori] {
  @import './../node_modules/fiori-fundamentals/scss/fonts';
  @import './../node_modules/fiori-fundamentals/scss/icons';
  @import './../node_modules/fiori-fundamentals/scss/core/forms.scss';
  @import './../node_modules/fiori-fundamentals/scss/core/elements.scss';
  @import './../node_modules/fiori-fundamentals/scss/layout';
  @import './../node_modules/fiori-fundamentals/scss/components';
  @import './../node_modules/fiori-fundamentals/scss/helpers';
}

By doing so you have no side effects by including the styles. You can opt-into Fiori Fundamentals anywhere just by writing:

<div class="fiori" data-fiori>
   <!-- anything in here will get all the fiori fundamentals goodness -->
</div>

This was also asked by myself in the last office hour and this is what I am using at the moment because I am facing a similar issue as OP.

[xak]

@ChristianKienle This is a drastic proposal that creates a burden on teams who only use FD.

The issue here is that FD default styles are clashing with other default styles. The problem here is default styles. We name-spaced all FD classes but we also include default styles, more aggressively that she prefer. Let's solve the problem with the default/reset styles.

My proposal to the FD team is to separate the default form element styles from the namespaced ones in core/forms ...

@mixin fd-input {
  //input rules
}
.fd-input {
  @include fd-input;
}
input[type=text], input[type=password], input[type=email], input[type=url], input[type=search], input[type=tel], input[type=number], input[type=date], input[type=time] {
  @include fd-input;
}

This will introduce some duplication of code but it will give teams the option to exclude the core/elements file in their own stylesheets. It is likely you would want to break it down further into core/reset, core/blocks, core/doc, etc. to make it easy to pick and choose what they want.

It's not unreasonable to expect a team using multiple libraries will need to manage their CSS imports more closely and deal with clashes here and there. With this setup, you could easily create the [data-fiori] shield in your own sheets if that works for you.

[ChristianKienle]

@ChristianKienle This is a drastic proposal that creates a burden on teams who only use FD.

The issue here is that FD default styles are clashing with other default styles. The problem here is default styles. We name-spaced all FD classes but we also include default styles, more aggressively that she prefer. Let's solve the problem with the default/reset styles.

My proposal to the FD team is to separate the default form element styles from the namespaced ones in core/forms ...

@mixin fd-input {
  //input rules
}
.fd-input {
  @include fd-input;
}
input[type=text], input[type=password], input[type=email], input[type=url], input[type=search], input[type=tel], input[type=number], input[type=date], input[type=time] {
  @include fd-input;
}

This will introduce some duplication of code but it will give teams the option to exclude the core/elements file in their own stylesheets. It is likely you would want to break it down further into core/reset, core/blocks, core/doc, etc. to make it easy to pick and choose what they want.

It's not unreasonable to expect a team using multiple libraries will need to manage their CSS imports more closely and deal with clashes here and there. With this setup, you could easily create the [data-fiori] shield in your own sheets if that works for you.

@xak I think you misunderstood my suggestion. The snippet I posted is nothing that should be included in Fundamentals. It is merely a workaround for @desaroxx 's problem.

I think his problem is that he has an app which is using some kind of styles which clash with the styles that come with Fundamentals. This makes it hard to him/his team to migrate to Fundamentals in small incremental steps because once he includes Fundamentals some styles are reset.

I obviously like your proposal very much. :) But this is not immediately available. The snippet posted above works now, without any changes in Fundamentals. I am aware that this is not the perfect solution but it allowed us to start the transition to Fundamentals.

[xak]

Ahh, sorry about that. I see what you mean. We're on the same page. 😄

[saad-mo]

@ChristianKienle I really like the approach that you described in your snippet. This is a common problem for teams who don't have the luxury of just using fundamentals in their project. Would you mind creating a wiki page with some more details of the problem and your proposal so that we can direct the teams to this page when they ask?

[ChristianKienle]

@ChristianKienle I really like the approach that you described in your snippet. This is a common problem for teams who don't have the luxury of just using fundamentals in their project. Would you mind creating a wiki page with some more details of the problem and your proposal so that we can direct the teams to this page when they ask?

If I get edit rights then yes. :)

[hisabimbola]

Hey, is there any update on this?

Sadly @ChristianKienle solution doesn't scale for us anymore. Currently in fundamental/angular while opening a modal. The open wrapper is dynamically generated and we can't apply the class and attribute. Would be nice to find a stable solution for this

Cc @droshev

[ChristianKienle]

@hisabimbola we are still using "my" approach (also with modals) to great effect. Maybe there is something not correct in your setup? If you want give me a call or ping me via slack.

[mpienkowski]

@ChristianKienle solution works like a charm, but there's one drawback with the font-sizes. Fundamental-styles 0.0.1 fd-has-type-x classes use REM units, which requires setting * { font-size: 14px}. We can't do that as it would break existing non-fiori components. The temporary workaround is to override fd-has-type-x classes font-sizes with pixel values.

A workaround:

[fiori] {
  font-size: 14px !important;

  .fd-has-type-minus-1 {
    font-size: 12px !important;
  }

  .fd-has-type, .fd-has-type-0 {
    font-size: 14px !important;
  }

  .fd-has-type-1 {
    font-size: 16px !important;
  }

  .fd-has-type-2 {
    font-size: 18px !important;
  }

  .fd-has-type-3 {
    font-size: 20px !important;
  }

  .fd-has-type-4 {
    font-size: 24px !important;
  }

  .fd-has-type-5 {
    font-size: 36px !important;
  }

  .fd-has-type-6 {
    font-size: 48px !important;
  }
}

[hisabimbola]

@ChristianKienle do you have that issue also because this looks messes and doesn't scale for us.