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(HelperClasses): add breakpoint helper classes #11786

Merged
merged 8 commits into from
Jul 29, 2022

Conversation

tw15egan
Copy link
Collaborator

Closes #2435

This PR adds in classes that can be used to hide elements only at specific breakpoints. This can already be done with the 0 column span grid classes, but this specifically addresses the need for this outside of a grid.

When the element is not in a grid, I wouldn't want to use grid classes.

2022-07-12 12 27 59

This is a PoC for a potential "Helper Classes" file that lives inside the utilities section of our scss package. We've gotten other requests for simple helper classes in the past, and this would give them a place to live. Would love to know what everyone thinks.

None of the classnames/filenames are set in stone and would love feedback.

For now, you can view the example in the storybook under the Helpers section at the bottom of the list.

Changelog

New

  • {{new thing}}

Changed

  • {{change thing}}

Removed

  • {{removed thing}}

Testing / Reviewing

{{ Add descriptions, steps or a checklist for how reviewers can verify this PR works or not }}

@netlify
Copy link

netlify bot commented Jul 12, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4e16f91
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/62e435334b271a000958efb1
😎 Deploy Preview https://deploy-preview-11786--carbon-components-react.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jul 12, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 4e16f91
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/62e435332eb7e00009cdfa63
😎 Deploy Preview https://deploy-preview-11786--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@tay1orjones tay1orjones 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 it makes sense to have a place like this for helper classes to live. A few thoughts:

  • Does it make sense to document these in an elements example instead of storybook?
  • Do we need to expose these via React Components? (Assuming no)
  • Should these be within a mixin (or maybe a mixin per-class?) so they're easier to compose within existing classes a product might have?
// use a single helper via mixins?
@use '@carbon/styles/scss/utilities/helper-classes';

.my-class {
  @include helper-classes.hide-at-sm();
}
// emit "hidden" helper classes and make them all available to this module via one big mixin?
@use '@carbon/styles/scss/utilities/helper-classes';

@include helper-classes.hidden();

@tw15egan
Copy link
Collaborator Author

tw15egan commented Jul 13, 2022

Does it make sense to document these in an elements example instead of storybook?

I don't have a preference here, but since it lives in the style package I just put it in the storybook.

Do we need to expose these via React Components? (Assuming no)

I don't think so

Should these be within a mixin (or maybe a mixin per -class?) so they're easier to compose within existing classes a product might have?

Good thinking, I think making them available as mixins fits better with the way we ship other helper utilities. I prefer the mixin per class approach, I can push up a PR with those changes so we can take a look

@tw15egan tw15egan force-pushed the breakpoint-helpers branch from d479160 to 6ca4ba2 Compare July 19, 2022 16:29
@tw15egan
Copy link
Collaborator Author

@tay1orjones I've just pushed some changes to move this to its own file called hide-at-breakpoint inside the utilities file. I've also gone ahead and updated the utilities section inside our style doc to help improve the discoverability of this and other helper mixins we have in there. Let me know what you think

@tw15egan tw15egan marked this pull request as ready for review July 19, 2022 16:48
@tw15egan tw15egan requested a review from a team as a code owner July 19, 2022 16:48
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

One small thing, otherwise looks good to me!

packages/styles/scss/utilities/_index.scss Show resolved Hide resolved
@tw15egan tw15egan force-pushed the breakpoint-helpers branch from 88dfe09 to f75988f Compare July 21, 2022 14:47
@tw15egan tw15egan requested a review from tay1orjones July 21, 2022 14:50
@tw15egan tw15egan force-pushed the breakpoint-helpers branch from e4e9884 to 3b9c5ef Compare July 21, 2022 15:20
@kodiakhq kodiakhq bot merged commit 53b6323 into carbon-design-system:main Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Grid] Add visibility classes
3 participants