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

Snackbar alignment fixed #4106

Merged
merged 3 commits into from Feb 15, 2016
Merged

Conversation

Rahul-Sagore
Copy link
Contributor

What is the expected result?
The Snackbar container should be center aligned, but it is not.

What happens instead of that?
It display it 50% from left + width of the container. You have used " left: 50% " css property to absolutely position the container to the 50% from the left. But the container will have its own width which will display it more ahead than 50% from left.

Bug ScreenShot: https://drive.google.com/file/d/0BxtQPnyvB1xpR2FSSDQ0MWkyTlk/view
See the weird position of snackbar message in above screenshot

Solution: I just did css changes in _snackbar.scss file, translating the container -50% from X axis, which will translate snackbar contaner -50% from left starting, making it absolutely center align.

@Rahul-Sagore
Copy link
Contributor Author

I was getting "Exception in thread "main" java.lang.UnsupportedClassVersionError: com/google/javascript/jscomp/CommandLineRunner : Unsupported major.minor version 51.0".

After running gulp test. My changes was just css based.

@Garbee Garbee added this to the 1.1.2 milestone Feb 12, 2016
@Garbee Garbee self-assigned this Feb 12, 2016
@Garbee
Copy link
Collaborator

Garbee commented Feb 12, 2016

Ignore the build failure please, looks like I may have introduced another bug by accident somewhere else. If that is the case then we can fix that and I'll handle merging the commit in manually.

I need to do some cross-browser testing here just to verify it works. I'll do that at some point this weekend and follow-up on the results.

Thanks for you time here, it is really appreciated.

@Garbee
Copy link
Collaborator

Garbee commented Feb 14, 2016

Verified working in Chrome and FF. Still need to test IE9, 10, and 11.

@sgomes Could you test this in Safari when you get the chance so we have verification it works as expected there?

@Rahul-Sagore
Copy link
Contributor Author

@Garbee Well, it should not make any difference in any browser, since I did not added new properties, instead I changed parameters in one of the existing property, that you have used.
It should not break as long as you are using browser prefixes with transform property (-moz-, -o-, -ms-, -webkit-).
Finger Crossed, since I know browser compatibility issues and life of front end developer!

@surma
Copy link
Contributor

surma commented Feb 15, 2016

LGTM

@sgomes
Copy link
Contributor

sgomes commented Feb 15, 2016

LGTM in all browsers I tested (including IE, Edge and Safari). Merging into master to avoid having to set up a new PR, then cherry-picking into mdl-1.1.

sgomes added a commit that referenced this pull request Feb 15, 2016
@sgomes sgomes merged commit 21293f3 into google:master Feb 15, 2016
@mfonville
Copy link

The problem with this patch is that the width of <div class=mdl-snackbar> can be an uneven number, if that is transformed with -50% in Google Chrome the font aliasing fails. :-(

@Rahul-Sagore
Copy link
Contributor Author

Hi, @Garbee , We forgot to add the code for mobile responsive. On mobile the snackbar should take full width, but it is translating -50% to the left from screen, as it should do this for desktop but not for mobile.
This code will work fine, hope you can add this:

    @media screen and (max-width: 480px){

      .mdl-snackbar--active {

         -webkit-transform: translatex(0) translatey(0);

         -moz-transform: translatex(0) translatey(0);

        -ms-transform: translatex(0) translatey(0);

        -o-transform: translatex(0) translatey(0);

         transform: translate(0, 0);
     }

   }

@Garbee
Copy link
Collaborator

Garbee commented Mar 17, 2016

iirc I already added that...

@Garbee
Copy link
Collaborator

Garbee commented Mar 17, 2016

Yea, PR #4179

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.

None yet

6 participants