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

feat(grid): add container component #128

Merged
merged 17 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions .storybook/blocks/Grid/Row.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import React, {
InputHTMLAttributes,
useCallback,
ChangeEvent,
ReactNode,
} from 'react';
import React, { ReactNode } from 'react';
import cn from 'classnames';

import styles from './Row.module.css';
Expand Down
28 changes: 14 additions & 14 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 4 additions & 10 deletions packages/grid/src/Grid.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { FC } from 'react';

import Col from './components/Col';
import { ColProps } from './components/Col.types';
import Container from './components/Container';
import Row from './components/Row';
import { RowProps } from './components/Row.types';

const Grid: {
Row: FC<RowProps>;
Col: FC<ColProps>;
displayName: string;
} = {
const Grid = {
Row,
Col,
Container,
displayName: 'Grid',
};
} as const;

export default Grid;
2 changes: 1 addition & 1 deletion packages/grid/src/__tests__/Col.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('Grid.Col', () => {
it('should set tag correcly', () => {
const dataTestId = 'test-id';
const { getByTestId } = render(
<Col Component="article" dataTestId={dataTestId} />,
<Col component="article" dataTestId={dataTestId} />,
);

expect(getByTestId(dataTestId).tagName).toBe('ARTICLE');
Expand Down
22 changes: 22 additions & 0 deletions packages/grid/src/__tests__/Container.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react';
import { render } from '@testing-library/react';

import Container from '../components/Container';

describe('Grid.Container', () => {
describe('Snapshots tests', () => {
it('should match snapshot', () => {
const { container } = render(<Container>content</Container>);

expect(container).toMatchSnapshot();
});
});

it('should set tag correctly', () => {
const { getByText } = render(
<Container component="section">hey</Container>,
);

expect(getByText('hey').tagName).toBe('SECTION');
});
});
2 changes: 1 addition & 1 deletion packages/grid/src/__tests__/Row.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('Grid.Row', () => {
it('should set tag correcly', () => {
const dataTestId = 'test-id';
const { getByTestId } = render(
<Row Component="section" dataTestId={dataTestId} />,
<Row component="section" dataTestId={dataTestId} />,
);

expect(getByTestId(dataTestId).tagName).toBe('SECTION');
Expand Down
11 changes: 11 additions & 0 deletions packages/grid/src/__tests__/__snapshots__/Container.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Grid.Container Snapshots tests should match snapshot 1`] = `
<div>
<div
class="container"
>
content
</div>
</div>
`;
30 changes: 0 additions & 30 deletions packages/grid/src/components/Col.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,6 @@
:root {
--grid-col-width: calc(100% / 12);
margin: 0 auto;

@media (--mobile) {
padding-right: var(--gap-l);
padding-left: var(--gap-l);
}
@media (--tablet) {
padding-right: var(--gap-xl);
padding-left: var(--gap-xl);
}

@media (--tablet-l) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop-m) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop-l) {
max-width: 1440px;
padding-right: var(--gap-9xl);
padding-left: var(--gap-9xl);
}
}

.component {
Expand Down
8 changes: 5 additions & 3 deletions packages/grid/src/components/Col.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ import { ColProps } from './Col.types';
import guttersStyles from '../styles/gutters.module.css';
import styles from './Col.module.css';

function Col({
Component = 'div',
function Col<E extends React.ElementType = 'div'>({
component,
className,
align,
order,
offset,
width,
children,
dataTestId,
}: ColProps) {
}: ColProps<E>) {
const Component = component ?? 'div';

const gridClassNames = useMemo(
() => getGridClassNames({ order, offset, width }, styles),
[order, offset, width],
Expand Down
10 changes: 7 additions & 3 deletions packages/grid/src/components/Col.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import { ResponsivePropertyType } from '../Grid.types';

export type ColProps = {
type ColBaseProps<E extends React.ElementType> = {
/**
* Additional class
*/
Expand Down Expand Up @@ -44,9 +44,10 @@ export type ColProps = {
order?: ResponsivePropertyType;

/**
* The html tag of the component
* The component used for the root node. Either a string to use a HTML element or a component
* @default "div"
*/
Component?: keyof JSX.IntrinsicElements;
component?: E;
Comment on lines -49 to +50
Copy link
Contributor Author

@frankie303 frankie303 Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: I think we should adjust other polymorphic components as well in a separate PR with this logic/change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Pagination will also use something similar


/**
* Content
Expand All @@ -58,3 +59,6 @@ export type ColProps = {
*/
dataTestId?: string;
};

export type ColProps<E extends React.ElementType> = ColBaseProps<E> &
Omit<React.ComponentProps<E>, keyof ColBaseProps<E>>;
38 changes: 38 additions & 0 deletions packages/grid/src/components/Container.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
@import '../../../themes/src/default.css';

.container {
width: 100%;
margin: 0 auto;
display: flex;
flex-direction: column;
justify-content: center;
box-sizing: border-box;
padding-left: var(--gap-l);
padding-right: var(--gap-l);

@media (--tablet) {
padding-right: var(--gap-xl);
padding-left: var(--gap-xl);
}

@media (--tablet-l) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop-m) {
padding-right: var(--gap-3xl);
padding-left: var(--gap-3xl);
}

@media (--desktop-l) {
max-width: 1440px;
padding-right: var(--gap-9xl);
padding-left: var(--gap-9xl);
}
}
26 changes: 26 additions & 0 deletions packages/grid/src/components/Container.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
import cn from 'classnames';

import { ContainerProps } from './Container.types';

import styles from './Container.module.css';

function Container<E extends React.ElementType = 'div'>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe C could represent container (also in Container.types.ts)?.

ElementType = 'div' could possibly be moved to the types file?

Another possible way to simplify a bit and transfer the typing logic to just one place, something like this could be done:

export interface PropsBasedOnComponent<T> {
  <C extends React.ElementType = 'div'>(
    props: {
      /**
       * The component used for the root node.
       * Either a string to use a HTML element or a component.
       */
      component?: C;
    } & T &
      Omit<React.ComponentPropsWithoutRef<C>, keyof T>,
  ): JSX.Element | null;
}

const Container: PropsBasedOnComponent<ContainerProps> = ({
  component,
  ...rest
}) => {

className,
children,
component,
...rest
}: ContainerProps<E>) {
const Component = component ?? 'div';

return React.createElement(
Component,
{
className: cn(styles.container, className),
...rest,
},
children,
);
}

export default Container;
13 changes: 13 additions & 0 deletions packages/grid/src/components/Container.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from 'react';

type ContainerBaseProps<E extends React.ElementType> = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very very nice typings \o/

/**
* The component used for the root node. Either a string to use a HTML element or a component
* @default "div"
*/
component?: E;
};

export type ContainerProps<E extends React.ElementType> =
ContainerBaseProps<E> &
Omit<React.ComponentProps<E>, keyof ContainerBaseProps<E>>;
9 changes: 6 additions & 3 deletions packages/grid/src/components/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { RowProps } from './Row.types';
import guttersStyles from '../styles/gutters.module.css';
import styles from './Row.module.css';

function Row({
Component = 'div',
function Row<E extends React.ElementType = 'div'>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing applies here, but like you've said on the PR we could have a proper PR to then unify this type of logic

component,
className,
gutter = {
mobile: 8,
Expand All @@ -24,11 +24,14 @@ function Row({
justify = 'between',
children,
dataTestId,
}: RowProps) {
}: RowProps<E>) {
const Component = component ?? 'div';

const gridClassNames = useMemo(
() => getGridClassNames({ gutter }, guttersStyles),
[gutter],
);

const classNames = cn(
guttersStyles.row,
styles.component,
Expand Down
Loading