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

fix(content-switcher): set height in button container instead of button #3758

Merged
merged 2 commits into from
Aug 19, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Aug 19, 2019

The latest code that dictates the height of content switcher buttons is introduced in #2085, whereas the height of their (the buttons') container is dictated by code from older commit. Given the issue #2085 refers to (#1956) seems to request simply for the button's height and doesn't seem to mention how buttons' height should work with their container's height, this change moves the code to define the height back to the container.

@aledavila Please don't hesitate to speak up if I'm missing anything!

Fixes #3751.

Changelog

Changed

  • A change to set content switcher button's height (40px) in their container, instead of in the buttons themselves.

Testing / Reviewing

Testing should make sure content switcher is not broken.

The latest code that dictates the height of content switcher buttons is
introduced in carbon-design-system#2085, whereas the height of their (the buttons')
container is dictated by code from older commit. Given the issue carbon-design-system#2085
refers to (carbon-design-system#1956) seems to request simply for the button's height and
doesn't seem to mention how buttons' height should work with their
container's height, this change moves the code to define the height
back to the container.

Fixes carbon-design-system#3751.
@netlify
Copy link

netlify bot commented Aug 19, 2019

Deploy preview for the-carbon-components ready!

Built with commit 3f34c7b

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

@netlify
Copy link

netlify bot commented Aug 19, 2019

Deploy preview for carbon-elements ready!

Built with commit 3f34c7b

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

@netlify
Copy link

netlify bot commented Aug 19, 2019

Deploy preview for carbon-components-react ready!

Built with commit 3f34c7b

https://deploy-preview-3758--carbon-components-react.netlify.com

Copy link
Member

@emyarod emyarod 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!

Copy link
Contributor

@aledavila aledavila 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 thanks for the fix akira

@asudoh asudoh merged commit 610dbb4 into carbon-design-system:master Aug 19, 2019
@asudoh asudoh deleted the content-switcher-height branch August 19, 2019 23:23
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.

ContentSwitcher has a defined height that is smaller than the containing buttons
3 participants