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: Align side-nav navigation arrow on IE11 #490

Merged

Conversation

salarenko
Copy link
Contributor

Aligning side-nav navigation arrow on IE11.

Related Issue

Closes #485

Description

Error occurred because of IE11 breaking align-items: center after setting min-height of an element. One of the workarounds enabling using min-height without breaking flex-box properties is to additionally set height < min-height.

More about the issue: philipwalton/flexbugs#231

Merge also includes updated side-nav playground template, providing updated html structure and 3 common side-navs usages.

@claassistantio
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Michal Salamon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@netlify
Copy link

netlify bot commented Dec 5, 2019

Deploy preview for fundamental-styles ready!

Built with commit ca81f98

https://deploy-preview-490--fundamental-styles.netlify.com

@salarenko salarenko changed the title Set height value lower than min-height to prevent align-items reset o… fix: Align side-nav navigation arrow on IE11 Dec 5, 2019
@JKMarkowski JKMarkowski force-pushed the fix/485-side-navigation-arrow-align-ie branch from 76ae625 to 04c23d9 Compare December 20, 2019 15:50
Comment on lines -10 to -15
<ul class="test-sidenav-list">
<li>Item 1</li>
<li>Item 1</li>
<li>Item 1</li>
<li>Item 1</li>
</ul>
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't bring the nested list in sidenav. There are tests for Nested list. Inside Sidenav tests we should only test the Sidenav styling because our components are self-contained. You should revert the test changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored previous test cases

@InnaAtanasova InnaAtanasova added this to the Sprint 28 - Sydney milestone Jan 6, 2020
@InnaAtanasova InnaAtanasova added this to In progress in Development via automation Jan 6, 2020
@InnaAtanasova InnaAtanasova moved this from In progress to Review in progress in Development Jan 6, 2020
Copy link
Contributor

@stefanoScalzo stefanoScalzo left a comment

Choose a reason for hiding this comment

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

LGTM just fix travis...very possible you may need to recheck ur visual regression tests

@stefanoScalzo stefanoScalzo self-requested a review January 9, 2020 15:35
Development automation moved this from PRs Review in progress to PRs Reviewer approved Jan 15, 2020
@InnaAtanasova InnaAtanasova merged commit 4011bab into SAP:master Jan 15, 2020
Development automation moved this from PRs Reviewer approved to Done Jan 15, 2020
@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development
  
Done
Development

Successfully merging this pull request may close these issues.

Side navigation broken in IE
6 participants