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

[LeftNav] Fix sidebar position for browsers that don't support transform3d #1269

Merged
merged 1 commit into from
Aug 27, 2015

Conversation

Andrew8xx8
Copy link
Contributor

LeftNav is not hidden in IE9. This small improvement solves problem for IE9 and probably for other browsers that don't support transfor3d. This improvement doesn't change the LeftNav behavior in modern browsers.

@pomerantsev
Copy link
Contributor

@Andrew8xx8 actually, with the fix, the behavior in modern browsers is quite different: the closing animation is broken, and the swipe to open feature doesn't work properly.
If you really need IE9 support, the only thing that I can think of (that shouldn't break anything else) is to try adding a different transform declaration before the 3d one.
Like:

transform: translateX(50px);
transform: translate3d(50px, 0, 0);

I don't remember whether it's possible to supply two values using material-ui's custom CSS-in-JS framework, and I'm not sure to what extent IE9 supports 2d transforms, but this may be something to try.

@hai-cea
Copy link
Member

hai-cea commented Aug 3, 2015

@Andrew8xx8 Is there any way to detect for IE9 and only apply your fix in that case?

@Andrew8xx8
Copy link
Contributor Author

@hai-cea Maybe we can use Modernizr.csstransforms3d here.


if (!Modernizr.csstransforms3d) {
styles.root.left = x + 'px';
}
Copy link
Member

Choose a reason for hiding this comment

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

You can move this into the style declaration:

left: Modernizr.csstransforms3d ? 0 : x + 'px',

@@ -101,7 +104,7 @@ let LeftNav = React.createClass({
width: this.getTheme().width,
position: 'fixed',
zIndex: 10,
left: 0,
left: Modernizr.csstransforms3d ? 0 : x + 'px',
Copy link
Member

Choose a reason for hiding this comment

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

Do we need px?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -101,7 +104,7 @@ let LeftNav = React.createClass({
width: this.getTheme().width,
position: 'fixed',
zIndex: 10,
left: 0,
left: Modernizr.csstransforms3d ? 0 : x,
Copy link
Member

Choose a reason for hiding this comment

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

I think you have to account for isBrowser as well:

left: isBrowser && Modernizr.csstransforms3d ? 0 : x,

@Andrew8xx8
Copy link
Contributor Author

@hai-cea fixed

hai-cea pushed a commit that referenced this pull request Aug 27, 2015
[LeftNav] Fix sidebar position for browsers that don't support transform3d
@hai-cea hai-cea merged commit 28403c3 into mui:master Aug 27, 2015
@hai-cea
Copy link
Member

hai-cea commented Aug 27, 2015

Thanks @Andrew8xx8 @pomerantsev !

@oliviertassinari
Copy link
Member

_setPosition doesn't benefit from this change and we should use translate2d instead.

@zannager zannager added the docs Improvements or additions to the documentation label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants