-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 align start and align end prop #17740
feat(grid): add align start and align end prop #17740
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17740 +/- ##
=======================================
Coverage 80.14% 80.15%
=======================================
Files 406 406
Lines 14030 14030
Branches 4391 4343 -48
=======================================
+ Hits 11245 11246 +1
+ Misses 2619 2618 -1
Partials 166 166 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
beforeEach(() => { | ||
jest.resetModules(); | ||
const FeatureFlags = require('@carbon/feature-flags'); | ||
FeatureFlags.enable('enable-css-grid'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC This is true by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be, without adding this however it was testing the flex grid version of the component. I realized after seeing the tests for the new prop fail and console.logging the output. I wonder if it could be avoided here if I import and test CSSGrid instead?
cleanup = require('@testing-library/react/pure').cleanup; | ||
render = require('@testing-library/react/pure').render; | ||
screen = require('@testing-library/react/pure').screen; | ||
Grid = require('../Grid').Grid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I'm curious, is this a new pattern we need to follow in certain situations? I know there's been some changes like this in best practices for @testing-library
but haven't seen this before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually pulled this code from the Column-test file when looking for an example of how we were enabling the feature flag within tests.
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
fa832a8
Hey there! v11.69.0 was just released that references this issue/PR. |
…17740) * feat(grid): add align start and align end prop * chore: revert changes --------- Co-authored-by: Gururaj J <89023023+Gururajj77@users.noreply.github.com>
Closes #17245
Add new align prop to Grid, update tests for Grid and FlexGrid
Changelog
New
align
propTesting / Reviewing
View default Grid story on wide monitor to see align start and align end examples. (revert story demo before merge)