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.

Sign up for GitHub

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

Jump to bottom

Rename Page Grid to Grid and make Grid Cell a subcomponent #684

Merged
merged 20 commits into from
Nov 1, 2023
Merged

Rename Page Grid to Grid and make Grid Cell a subcomponent #684

merged 20 commits into from
Nov 1, 2023

Conversation

VincentSmedinga
Copy link
Contributor

No description provided.

alimpens
alimpens reviewed Oct 27, 2023
View reviewed changes
packages/react/src/Grid/GridCell.tsx Outdated Show resolved Hide resolved
packages/react/src/Grid/index.ts Outdated Show resolved Hide resolved
packages/css/src/grid-cell/grid-cell.scss Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 27, 2023 14:20 Destroyed
@VincentSmedinga
Merge branch 'main' into feature/DES-335-rename-page-grid
8727ba7 
# Conflicts:
#	storybook/storybook-react/src/Highlight/Highlight.stories.tsx
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 27, 2023 18:56 Destroyed
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 30, 2023 13:05 Destroyed
@VincentSmedinga VincentSmedinga requested a review from dlnr October 30, 2023 13:07
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 30, 2023 14:00 Destroyed
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 30, 2023 14:08 Destroyed
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 30, 2023 14:13 Destroyed
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 30, 2023 14:16 Destroyed
dlnr
dlnr reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@dlnr dlnr left a comment

Choose a reason for hiding this comment

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

The rename to the compounded component feels like it's not consistent with other components that we've grouped like this, because it's not really a component like the others. So I have doubts about the structure in general.

packages/css/src/grid/README.grid-cell.md Outdated Show resolved Hide resolved
packages/css/src/grid/grid-cell.scss Show resolved Hide resolved
packages/css/src/grid/grid.scss Outdated Show resolved Hide resolved
packages/css/src/index.scss Show resolved Hide resolved
packages/css/src/screen/README.md Show resolved Hide resolved
packages/react/src/Grid/Grid.test.tsx Outdated Show resolved Hide resolved
packages/react/src/Grid/Grid.tsx Show resolved Hide resolved
packages/react/src/Grid/GridCell.test.tsx Outdated Show resolved Hide resolved
storybook/storybook-react/config/preview.tsx Show resolved Hide resolved
storybook/storybook-docs/src/grid.stories.mdx Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 31, 2023 13:40 Destroyed
@github-actions github-actions bot temporarily deployed to staging-feature-DES-335-rename-page-grid October 31, 2023 15:01 Destroyed
@VincentSmedinga VincentSmedinga merged commit 3d25409 into main Nov 1, 2023
3 checks passed
@VincentSmedinga VincentSmedinga deleted the feature/DES-335-rename-page-grid branch November 1, 2023 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alimpens alimpens alimpens left review comments

@dlnr dlnr dlnr left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VincentSmedinga @dlnr @alimpens