Skip to content

fix(grid): make subgrid work with grid offset#9803

Merged
kodiakhq[bot] merged 3 commits into
carbon-design-system:mainfrom
janhassel:subgrid-offset
Oct 13, 2021
Merged

fix(grid): make subgrid work with grid offset#9803
kodiakhq[bot] merged 3 commits into
carbon-design-system:mainfrom
janhassel:subgrid-offset

Conversation

@janhassel

Copy link
Copy Markdown
Member

Ref #9723 (comment)

Ensures that subgrid is supported corectly when the parent grid uses offset.

Changelog

Removed

  • Removed explicit col-end-n rules since the existing col-span-n and col-start-n classes together are already achieving the expected styling

Testing / Reviewing

Add the test case from the discussion as a story and verify the appearance / behaviour:

export const Test = () => {
  return (
    <Grid>
      <Column sm={4} md={{ span: 6, offset: 1 }} lg={{ span: 8, offset: 1 }}>
        <div style={{ background: 'gray' }}>Full</div>
        <Grid>
          <Column sm={2} md={3} lg={4}>
            <div style={{ background: 'gray' }}>Half</div>
          </Column>
          <Column sm={2} md={3} lg={4}>
            <div style={{ background: 'gray' }}>Half</div>
          </Column>
          <Column sm={2} md={3} lg={4}>
            <div style={{ background: 'gray' }}>Half</div>
          </Column>
          <Column sm={true} md={true} lg={true}>
            Auto col
          </Column>
        </Grid>
      </Column>
    </Grid>
  );
};

@janhassel janhassel requested a review from a team as a code owner October 5, 2021 15:12
@janhassel janhassel requested review from aledavila and dakahn October 5, 2021 15:12
@netlify

netlify Bot commented Oct 5, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: f0798d9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/616725b697e0590007f1c455

😎 Browse the preview: https://deploy-preview-9803--carbon-react-next.netlify.app

@tay1orjones tay1orjones self-requested a review October 5, 2021 15:52
@netlify

netlify Bot commented Oct 5, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: f0798d9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/616725b69572990007f25180

😎 Browse the preview: https://deploy-preview-9803--carbon-components-react.netlify.app

@netlify

netlify Bot commented Oct 5, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: f0798d9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/616725b55a3c7000096febbf

😎 Browse the preview: https://deploy-preview-9803--carbon-elements.netlify.app

@tay1orjones tay1orjones left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have one question on my review below, but both your example story and the existing offset story look great.

proper.offset.column.reflow.mov

I pushed one small update to this - I noticed the existing stories' styles weren't being applied due to the prefix change.

Comment thread packages/grid/scss/modules/_css-grid.scss

@dakahn dakahn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@kodiakhq kodiakhq Bot merged commit 9f558cf into carbon-design-system:main Oct 13, 2021
@janhassel janhassel deleted the subgrid-offset branch October 15, 2021 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants