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

[Carbon X] Implicit height on Button can sometimes render as zero height #1691

Closed
jendowns opened this issue Jan 28, 2019 · 5 comments
Closed

[Carbon X] Implicit height on Button can sometimes render as zero height #1691

jendowns opened this issue Jan 28, 2019 · 5 comments
Assignees
@emyarod

Comments

@jendowns
Copy link
Contributor

Detailed description

Button height for Carbon X version is implicit:
https://github.com/IBM/carbon-components/blob/master/src/components/button/_mixins.scss#L84

This usually doesn't cause issues for most buttons, since they typically have content within.

However, the ButtonSkeleton from carbon-component-react renders with a zero height due to this implicit height: auto setting.

Suggested Fix

Adding a pixel value for height resolves in the Carbon X button base mixin resolves this issue.
@jendowns
Copy link
Contributor Author

I'm happy to propose a fix to this; it would be a short PR 😅

However I've noticed on some of our mockups that buttons are typically 48px. Whereas the current Carbon version has $button-height set to 40px.

So do you mind if I set height: rem(48px); in the button--x mixin, if that's the ultimate intention?

@jendowns jendowns changed the title [Carbon X]: Implicit height on Button can sometimes render as zero height [Carbon X] Implicit height on Button can sometimes render as zero height Jan 29, 2019
@asudoh
Copy link
Contributor

asudoh commented Jan 30, 2019

CC @IBM/carbon-designers to see which one would be the correct height. Thanks!

@aagonzales
Copy link
Member

aagonzales commented Jan 31, 2019 •
edited
Loading

Carbon X button heights are 48px. But V9 they are 40px.

@asudoh
Copy link
Contributor

asudoh commented Feb 1, 2019

Thanks @aagonzales for your response!

@emyarod emyarod self-assigned this Feb 1, 2019
@emyarod emyarod mentioned this issue Feb 1, 2019
Merged
@carbon-bot
Copy link
Contributor

🎉 This issue has been resolved in version 9.70.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

joshblack pushed a commit to joshblack/carbon that referenced this issue May 2, 2019
@emyarod
feat(Tile): add experimental tile (carbon-design-system#1691)
2485b55 
* feat(RadioTile): support keyboard navigation

* feat(Tile): support keyboard navigation

* feat: use new icon strategy

* docs(Tile): prevent page navigation on click of ClickableTile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emyarod emyarod

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@asudoh @emyarod @jendowns @aagonzales @carbon-bot