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): enable zero span columns #5052

Merged
merged 12 commits into from
Jan 20, 2020
Merged

feat(Grid): enable zero span columns #5052

merged 12 commits into from
Jan 20, 2020

Conversation

zachhardesty7
Copy link
Contributor

Adds functionality requested in (#2435) with more convenient syntax.

Since Grid components may often have their classes added programatically, enabling the use of span "0" simplifies the code and improves readability. From an English language syntax perspective, content "spanning 0 columns" makes sense in a similar way as content with a "width of 0." Additionally, several other major libraries support this notation.

Changelog

New

  • Extend SCSS to generate class .bx--col-0 that sets display: none on a given column

Testing / Reviewing

The example docs now include a demonstration of the class in action!

Adds functionality requested in (#2435) with more convenient syntax.

Originally from PR #4894.
@netlify
Copy link

netlify bot commented Jan 15, 2020

Deploy preview for the-carbon-components ready!

Built with commit e7f03b1

https://deploy-preview-5052--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 15, 2020

Deploy preview for carbon-elements ready!

Built with commit e7f03b1

https://deploy-preview-5052--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Jan 15, 2020

Deploy preview for carbon-components-react failed.

Built with commit e7f03b1

https://app.netlify.com/sites/carbon-components-react/deploys/5e262b5158e14f000ae152ea

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Looks basically good, wanted to see what @joshblack thinks. Great to have some clarification of for loop starting -1 (Is it some kind of Sass trick?). Thanks @zachhardesty7!

@zachhardesty7
Copy link
Contributor Author

@asudoh Honestly, I wrote this quite a while ago as a feature of the (upcoming) React Grid components. I have no idea why I put -1 instead of 0. I tested it with both and -1 indeed allows an incorrect span. I fixed it here: 7354c74

packages/grid/scss/_mixins.scss Outdated Show resolved Hide resolved
@asudoh asudoh requested review from a team and aagonzales and removed request for a team January 18, 2020 00:08
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

I think LGTM.

Copy link
Contributor

@abbeyhrt abbeyhrt left a 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! Thank you for doing this!

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.

5 participants