Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change all utility classes to match 8pix grid #69

Open
MagdalenaLarge opened this issue Jul 10, 2024 · 5 comments · Fixed by ONSdigital/design-system#3285
Open

Change all utility classes to match 8pix grid #69

MagdalenaLarge opened this issue Jul 10, 2024 · 5 comments · Fixed by ONSdigital/design-system#3285
Assignees
Labels
Enhancement Change of existing feature High Priority In development To make the issue appear in 'working on now' on the Service Manual ONS website

Comments

@MagdalenaLarge
Copy link

MagdalenaLarge commented Jul 10, 2024

As Interaction Designer
I want to change all utility classes to match 8 pix grid
So that it is easier to design content of the ONS website

Description
It was highlighted by Interaction Designers that majority of Design System components are set up with 9 pix grid.
However there is no consistency and we also identified some with 8pix.
In preparation for the ONS website, we need to standardise all utility classes to match 8 pix grid.
This change will improve the quality of the Design System.

Acceptance Criteria

  1. All the utility classes have with 9 pix has been identified
  2. All utility classes has been changed to 8 pix grid

Visualisation/Input from UCD
How should the components look like?
Linked User stories/epics
Technical details

All we need to change is the _typography file within the vars folder with the below :

$base: 16px;

$type-matrix: (
    ons-u-fs-xxxl: (
        small: 32px,
        large: 48px,
        weight: $font-weight-bold,
        line-height-mobile: 44px,
        line-height-desktop: 56px
    ),
    ons-u-fs-xxl: (
        small: 28px,
        large: 36px,
        weight: $font-weight-bold,
        line-height-mobile: 40px,
        line-height-desktop: 48px
    ),
    ons-u-fs-xl: (
        small: 26px,
        large: 30px,
        weight: $font-weight-bold,
        line-height-mobile: 36px,
        line-height-desktop: 40px
    ),
    ons-u-fs-l: (
        small: 24px,
        large: 26px,
        weight: $font-weight-bold,
        line-height-mobile: 32px,
        line-height-desktop: 36px
    ),
    ons-u-fs-m: (
        small: 20px,
        large: 22px,
        weight: $font-weight-bold,
        line-height-mobile: 28px,
        line-height-desktop: 32px
    ),
    ons-u-fs-r--b: (
        small: 18px,
        large: 18px,
        weight: $font-weight-bold,
        line-height-mobile: 28px,
        line-height-desktop: 28px
    ),
    ons-u-fs-r: (
        small: 18px,
        large: 18px,
        weight: $font-weight-regular,
        line-height-mobile: 28px,
        line-height-desktop: 28px
    ),
    ons-u-fs-s--b: (
        small: 14px,
        large: 14px,
        weight: $font-weight-bold,
        line-height-mobile: 20px,
        line-height-desktop: 20px
    ),
    ons-u-fs-s: (
        small: 14px,
        large: 14px,
        weight: $font-weight-regular,
        line-height-mobile: 20px,
        line-height-desktop: 20px
    ),
);

And we also need to update the typography file in the utilities folder with the below :

@mixin font-size($props, $base) {
    $small-size: map.get($props, small);
    $large-size: map.get($props, large);

    font-size: rems($small-size, $base);
    font-weight: map.get($props, weight);
    line-height: map.get($props, line-height-mobile);

    @if $small-size != $large-size {
        @include mq(m) {
            font-size: rems($large-size, $base);
            line-height: map.get($props, line-height-desktop)
        }
    }
}

This will also update the line height value for mobile and desktop view
MVP (optional)

@MagdalenaLarge
Copy link
Author

Screenshot 2024-07-12 at 10 07 26
For the reference please use the Typography advised by Interaction Designers.

@SriHV SriHV self-assigned this Jul 15, 2024
@MagdalenaLarge
Copy link
Author

Ticket will incorporate the work described in ticket #73

@Majsol
Copy link

Majsol commented Jul 17, 2024

Image

@alessioventuriniAND
Copy link
Collaborator

@Majsol and Gareth will need to look into margin and padding sizing as the change of the root font size to 16px will not only impact the font size but also the computed value of rems which is the unit we use for our margin and paddings.

@e-livingstone e-livingstone added the In development To make the issue appear in 'working on now' on the Service Manual label Aug 1, 2024
@alessioventuriniAND
Copy link
Collaborator

alessioventuriniAND commented Aug 1, 2024

Update on this work :

I have replaced the below:

margin-top
margin-left
margin-right
margin-bottom

with the relevant margin utility class. I have also added the new margin and padding sizes and replace the old utility classes across the repo with the new correspondent ones.

//NEW Sizes 
$sizes: (
    no: 0,
    4xs: 0.125rem,
    3xs: 0.25rem,
    2xs: 0.5rem,
    xs: 0.75rem,
    s: 1rem,
    m: 1.25rem,
    l: 1.5rem,
    xl: 2rem,
    2xl: 2.5rem,
    3xl: 3rem,
    4xl: 4rem,
    5xl: 5rem,
    auto: auto,
);

I have also updated our font size type matrix to match the new 8px grid. By changing our root font size to 16px our rems are now calculated using the 8px grid. I have also set the body font size to 18px as per the new type matrix.

I have updated all the shorthand margin declaration with the closest size listed in our sizing grid. We should look to introduce vertical and horizontal margin and padding utility classes that we can replace all our shorthand margin with the relevant utility class.

Padding has been decided not to be touched for now. I have however also replace the old utility classes with the new correct size.

Line height has been updated as per our new grid.

Considerations.

We can't extend classes inside our media queries so we will have margin properties manually set inside media queries.

@alessioventuriniAND alessioventuriniAND linked a pull request Aug 7, 2024 that will close this issue
2 tasks
@alessioventuriniAND alessioventuriniAND removed their assignment Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Change of existing feature High Priority In development To make the issue appear in 'working on now' on the Service Manual ONS website
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants