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(style/--side-nav__navigation): remove css height to allow components at bottom (#5871) #12152

Conversation

gi
Copy link
Contributor

@gi gi commented Sep 26, 2022

This removes the CSS height style from --side-nav__navigation. The resulting style for this element when composed with --side-nav is:

position: fixed;
top: #{header-height};
bottom: 0;

With fixed positioning and top and bottom specified, the CSS height is unnecessary.

Additionally, the CSS height: 100% calculates the height from the body, which is more than the viewport height. Thus, the height of the element will extend beyond the viewport, 'below the fold".

The element --side-nav__items, included as a child element, has flex-grow set to 1, causing it to fill the entire height.

Any element included after the side nav items in the side nav are pushed to the bottom. They should be expected to show entirely within the viewport, yet since the bottom is below, they either are not shown or are chopped off.

This fixes this issue.

Note: This may or may not solve #5871. It is not intended as a full solution, but it does fix this styling issue which would prevent properly supporting such a feature.

Changelog

Removed

  • style/--side-nav__navigation: height removed

Testing / Reviewing

  • Tested visually

* fix(UIShell/SideNav): remove css height to allow components at bottom
@gi gi requested a review from a team as a code owner September 26, 2022 02:16
@netlify
Copy link

netlify bot commented Sep 26, 2022

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit d07ae0a
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/63330d1364dc350008ba6dd6
😎 Deploy Preview https://deploy-preview-12152--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 Sep 26, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit d07ae0a
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/63330d137eb43d00082003bd
😎 Deploy Preview https://deploy-preview-12152--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

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing another fix 👍 ✅

@kodiakhq kodiakhq bot merged commit 2a2c8a6 into carbon-design-system:main Sep 27, 2022
@gi gi deleted the issue-5871/ui-shell-left-nav/remove-css-height branch September 27, 2022 23:10
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.

3 participants