feat: introduce Margins and Paddings

[kavya-b]

Related Issue

Closes #2119

Description

This PR introduces helper modifier classes that can be added to any element that needs some spacings through paddings or margins.

Screenshots

Before:

NA

After:

Margins:

Paddings:

Please check whether the PR fulfills the following requirements

1. The output matches the design specs

• All values are in rem
• [n/a] Text elements follow the truncation rules
• [n/a] hover state of the element follow design spec
• [n/a] focus state of the element follow design spec
• [n/a] active state of the element follow design spec
• [n/a] selected state of the element follow design spec
• [n/a] selected hover state of the element follow design spec
• [n/a] pressed state of the element follow design spec
• Responsiveness rules - the component has modifier classes for all breakpoints
• Includes Compact/Cosy/Tablet design
• RTL support

2. The code follows fundamental-styles code standards and style

• only one top level fd-* class is used in the file
• BEM naming convention is used
• Mixins are used for repeatable code (fd-rtl, fd-ellipsis, fd-flex, fd-selected, fd-focus, ect.)
• A11y support - keyboard support, screenreader support, proper ARIA attributes, etc.
• [n/a] fd-reset() mixin is applied to all elements
• Variables are used, if some value is used more than twice.
• Checked if current components can be reused, instead of having new code.

3. Testing

• tested Storybook examples with "CSS Resources" normalize option
• tested Storybook examples with "CSS Resources" unnormalize option
• Verified all styles in IE11
• Updated tests
• last commit message should have [ci visual] so it can trigger chromatic visual regression (e.g. test: run chromatic visual regression [ci visual])

4. Documentation

• Storybook documentation has been created/updated
• [n/a] Breaking Changes wiki has been updated in case of breaking changes.

[netlify]

Deploy preview for fundamental-styles ready!

Built with commit 00bc87c

https://deploy-preview-2131--fundamental-styles.netlify.app

[kavya-b]

One thing I am not sure of is RTL handling. For the case of No Margins, if we have a margin of say 1.5rem initially, fd-margin-end--none will give margin-right: 0 !important; which is expected, but switching to RTL makes it margin-left: 0 !important;, but there is no way to actually retrieve the initial value of 1.5rem that the user set.

in RTL becomes

because the margin-right value was already set to 0, and the original value is lost. How do we handle this?

[InnaAtanasova]

One thing I am not sure of is RTL handling. For the case of No Margins, if we have a margin of say 1.5rem initially, fd-margin-end--none will give margin-right: 0 !important; which is expected, but switching to RTL makes it margin-left: 0 !important;, but there is no way to actually retrieve the initial value of 1.5rem that the user set.

in RTL becomes

because the margin-right value was already set to 0, and the original value is lost. How do we handle this?

would margin-right: initial; work?

[kavya-b]

One thing I am not sure of is RTL handling. For the case of No Margins, if we have a margin of say 1.5rem initially, fd-margin-end--none will give margin-right: 0 !important; which is expected, but switching to RTL makes it margin-left: 0 !important;, but there is no way to actually retrieve the initial value of 1.5rem that the user set.

in RTL becomes

because the margin-right value was already set to 0, and the original value is lost. How do we handle this?

would margin-right: initial; work?

I tried initial, unset, inherit, revert, auto with !important but none of them worked. Also, most of these don't work with IE anyway.

[JKMarkowski]

Hello @kavya-b Could you try to add some mixin, which will be named fd-not-rtl, or fd-ltr? Maybe you can combine selector ::not() with RTL.
So for example margin-right will be modified only when there is not RTL, which means you won't need to reset margin-right on RTL

[kavya-b]

Hello @kavya-b Could you try to add some mixin, which will be named fd-not-rtl, or fd-ltr? Maybe you can combine selector ::not() with RTL.
So for example margin-right will be modified only when there is not RTL, which means you won't need to reset margin-right on RTL

Hi @JKMarkowski that is a good idea and I have been trying this, but unfortunately it isn't working as expected.
I applied not() in the following way:

&--none {
      :not([dir="rtl"]), :not([dir="rtl"]) &{
        margin-right: 0 !important;
      }

      @include fd-rtl() {
        margin-left: 0 !important;
      }
    }

:not([dir="rtl"]) .fd-margin-end--none somehow does not work as intended in RTL.

:not([dir="rtl"] .fd-margin-end--none) works, but is incorrect as it affects everything that is not .fd-margin-end--none.

Apparently, the :not() does not work very well with complex selectors and has limited browser support as well.

[kavya-b]

@InnaAtanasova @JKMarkowski since I was not able to get around the RTL issue for fd-margin-begin--none and fd-margin-end--none through the lib styles itself, I am now applying a workaround from the docs side, where I set the [dir="rtl"] for the user-specified class in customs.scss, while also removing !important from the corresponding lib modifier. Please take a look if this is okay for now.

[JKMarkowski]

@kavya-b I also wasn't able to combine at-root / rtl with not. The other thing is to force users to put [dir="ltr"] by default, but this is something that everyone should agree on
@InnaAtanasova What do you think about it?

[kavya-b]

@kavya-b I also wasn't able to combine at-root / rtl with not. The other thing is to force users to put [dir="ltr"] by default, but this is something that everyone should agree on
@InnaAtanasova What do you think about it?

Hi @InnaAtanasova what are your thoughts about this?

[InnaAtanasova]

@kavya-b I also wasn't able to combine at-root / rtl with not. The other thing is to force users to put [dir="ltr"] by default, but this is something that everyone should agree on
@InnaAtanasova What do you think about it?

Hi @InnaAtanasova what are your thoughts about this?

@kavya-b I need to discuss this with my team but from my research I found out that other libraries do not consider additional paddings and margins applied on the element. Most of the other libraries do not apply RTL at all. I will write back to you when we have some decision about it.

[InnaAtanasova]

@kavya-b we just discussed the issue and we agreed that any additional padding/margins applied on the element should be ignored.

[InnaAtanasova]

Code looks good, I am just worried the documentation is a bit confusing.
This image is illustrating the margins:

But usually this is the way to illustrate the paddings. The containers should all have the same size and the margins are "outside" the containers. Something like this:

Same for paddings. This is how it's now in the documentation:

But it's the way of illustrating margins. This is how it should be:

I believe the best way would be to make all containers the same size and play with the backgrounds and maybe add borders to show the paddings and the margins.

[kavya-b]

Code looks good, I am just worried the documentation is a bit confusing.
This image is illustrating the margins:

But usually this is the way to illustrate the paddings. The containers should all have the same size and the margins are "outside" the containers. Something like this:

Same for paddings. This is how it's now in the documentation:

But it's the way of illustrating margins. This is how it should be:

I believe the best way would be to make all containers the same size and play with the backgrounds and maybe add borders to show the paddings and the margins.

@InnaAtanasova
margins now look like this:

and paddings look like this:

hope this is okay.

[InnaAtanasova]

@kavya-b looks great! Thanks for updating the docs <3

[JKMarkowski]

LGTM. Great Job!