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

Dev: add grid gap property for more flexibility #1106

Open
3 tasks
adorayi opened this issue Aug 27, 2024 · 8 comments
Open
3 tasks

Dev: add grid gap property for more flexibility #1106

adorayi opened this issue Aug 27, 2024 · 8 comments
Assignees

Comments

@adorayi
Copy link
Contributor

adorayi commented Aug 27, 2024

Problem

On certain viewports, the spacing for the grid gap is too much. Need to override with css (see below). We need to allow changing the spacing values between containers on the grid component.

image (5).png

Site.css:

 @media (max-width: 600px) {
    gcds-grid {
        --gcds-grid-gap: 0;
    }

    gcds-text {
        text-align: left;
    }
}
<gcds-grid tag="article" columns-desktop="1fr" columns-tablet="1fr" columns="1fr">
    <gcds-container class="new-class" size="xs" centered margin="0" padding="0">
   This is some example content to display the grid component.
    </gcds-container>
    <gcds-container  size="xs" centered margin="0" padding="0">
   This is some example content to display the grid component
    </gcds-container>
    <gcds-container  size="xs" centered margin="0" padding="0">
   This is some example content to display the grid component
    </gcds-container>
</gcds-grid>

Acceptance criteria

  • Add new <gap> properties that allow users to change the value of the gap depending on the viewport and adhere to the vertical rhythm of the new typography.
  • The new allowed values are <gcds-soacing-150> to <gcds-spacing-800>.
  • The new default gap value for all viewports is <gcds-spacing-300>.

Definition of done

The new gap, gapTablet, and gapDesktop properties have been added and provide the values <gcds-soacing-150> to <gcds-spacing-800> as options.

@adorayi adorayi added For Grooming Issues with this label will be prioritized in the next team grooming session and likely next sprint High Priority | Haute priorité labels Aug 27, 2024
@melaniebmn
Copy link

Hi @RavinaSamaroo let's chat about this next week when you are back :)

@adorayi
Copy link
Contributor Author

adorayi commented Sep 4, 2024

Consider the new spacing component on the Figma side.
Consider having a dedicated page on Figma for the spacing component, and referencing it on the grid page.

@adorayi
Copy link
Contributor Author

adorayi commented Oct 1, 2024

Needs decision documented and to be considered together with spacing component.

@ClementineHahn
Copy link
Collaborator

Initial discussion with proposed next steps was there with @melaniebmn and @realdylanzheng , has to be documented here before going back into product backlog

@realdylanzheng
Copy link

realdylanzheng commented Nov 15, 2024

Proposed solution:

  • Give designers and developers the flexility to choose from a curated set of spacing values for consistent padding between containers within the grid component.
  • The chosen spacing value will be applied uniformly to both horizontal and vertical spacing to maintain a cohesive visual rhythm across the grid layout.
  • The offered spacing values will allow for clear grouping of elements and maintain perceptible relationships within the grid. (ensuring law of proximity). To achieve this the spacing values should range from the snapped line heights of the smallest font size to the largest font size. +/- one interval
  • NEW range of spacing values after typography & spacing updates have been completed: gcds-spacing-150 - gcds-spacing-600
    @melaniebmn to review and confirm thanks

Note: old proposed range of spacing values: gcds-spacing-200 - gcds-spacing-500
(as per the new spacing values from the Typography and spacing specs doc)

@ClementineHahn
Copy link
Collaborator

Could be added into the Sprint backlog but needs better refinement first on def of done @daine

@melaniebmn
Copy link

melaniebmn commented Jan 9, 2025

Update:

Adjustment to the spacing range after our typography updates. The new range will include the values gcds-spacing-150 - gcds-spacing-800 (including the new values we have added within that range) which replaces the old gcds-spacing-200 - gcds-spacing-500 range.

@melaniebmn melaniebmn changed the title Issue with grid spacing on certain view ports Dev: add grid gap property for more flexibility Jan 13, 2025
@melaniebmn
Copy link

The new gap properties have been added in this PR.

@ClementineHahn ClementineHahn added development Development tasks and removed For Grooming Issues with this label will be prioritized in the next team grooming session and likely next sprint labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants