mdbootstrap / mdb-ui-kit

Bootstrap 5 & Material Design UI KIT
https://mdbootstrap.com/docs/standard/
Other
24.2k stars 3.52k forks source link

Regression on master - No ripple effect without form-group for checkbox and radio #980

Closed tilwinjoy closed 8 years ago

tilwinjoy commented 8 years ago

Hi,

The ripple effect is not working without .form-group for checkbox and radio. I'm using chrome on Ubuntu machine. (I don't think it's platform/browser specific issue since the official demos for checkbox and [radio](http://fezvrasta.github.io/bootstrap-material-design/#radio-button and) has ripple effect inside and outside .form-group on the same machine and browser).

Outsitde form-group test case - doesn't work (different than sample site) Inside form-group test case - works

rosskevin commented 8 years ago

Perhaps you should start with our template?

https://github.com/FezVrasta/bootstrap-material-design/blob/master/CONTRIBUTING.md#tldr-create-a-test-case-using-this-codepen-template-when-submitting-an-issue

tilwinjoy commented 8 years ago

@rosskevin updated with the template. The issue still seems to be there...

rosskevin commented 8 years ago

Curious, are you using a bower/npm release or master? (codpen points to master, so it may be a recent regression)

tilwinjoy commented 8 years ago

@rosskevin In the project I'm using bower couldn't make it work. That's why I tried to make it work with minimal code in the jsfiddle I shared earlier. The codepen points to latest code as you said. Wondering how it works in the git hub demo page... it's points to dist/css/bootstrap-material-design.css and dist/js/material.js any idea which version they are? I checked the file but the version is not there...

rosskevin commented 8 years ago

Demo site is using the gh-pages branch, which is the last released version 0.5.9. You can pull from the tag: https://github.com/FezVrasta/bootstrap-material-design/releases

That confirms this as a regression.

niravpatel9898 commented 8 years ago

@tilwinjoy Ctrf + F the below line in bootstrap-material-design.css:

.checkbox input[type=checkbox]:checked + .checkbox-material:before

change its css to the one given below, just give it a shot

.checkbox input[type=checkbox]:checked + .checkbox-material:before {
  -webkit-animation: rippleOn 500ms;
       -o-animation: rippleOn 500ms;
          animation: rippleOn 500ms;
}
rosskevin commented 8 years ago

@tilwinjoy I committed a grunt dist so the test cases above are using your last pull. I was just curious if it changed anything regarding ripple but it didn't. I'm a novice on animations.

@FezVrasta can you take a quick look at the test cases at the top? The dist is up-to-date.

FezVrasta commented 8 years ago

I think I'm using form-group as wrapper for the radio/toggle and its JS added ripple. without it, the ripple element is not properly placed...

tilwinjoy commented 8 years ago

@rosskevin I believe it has to do with the removal of forwards property and duration from the animations (they were present in the working version). I'll look into it.

tilwinjoy commented 8 years ago

@rosskevin The issue occurred due to the below changes, reverting it adds the animation back - updated testcase

However it looks like this changes were made so that the animation doesn't play on page load. In that case I'll look into an alternative solution (Maybe will end up controlling the animation on click via js?).

checkbox

duration and fill-mode 0.3s forwards removed from

 .check:before {
      ...
      animation: checkbox-off 0.3s forwards;
    }

duration 500ms;removed from

    & + .checkbox-material:before {
        animation: rippleOn 500ms;
      }

and

&:not(:checked) {
      & + .checkbox-material:before {
        animation: rippleOff 500ms;
      }

in favour of

// Prevent checkbox animation and ripple effect on page load
.is-focused {
  .checkbox {
    .checkbox-material {
      .check:before {
        animation: checkbox-off @mdb-checkbox-animation-check forwards;
      }
    }
    input[type=checkbox] {
      &:checked {
        & + .checkbox-material:before {
          animation: rippleOn @mdb-checkbox-animation-ripple;
        }
      }
      &:not(:checked) {
        & + .checkbox-material:before {
          animation: rippleOff @mdb-checkbox-animation-ripple;
        }
      }
    }
  }
}

radio:

duration 500ms removed from

input[type=radio]:checked ~ .check:after {
      animation: rippleOn 500ms;
    }

In favor of

// Prevent ripple effect on page load
.is-focused {
  .radio {
    input[type=radio]:checked ~ .check:after {
      animation: rippleOn 500ms;
    }
  }
}
rosskevin commented 8 years ago

Yes, it's definitely a plus not to have them play on page load - that's a real annoyance. I'd rather force use of form-group than revert that.

tilwinjoy commented 8 years ago

@rosskevin @FezVrasta Since we are talking about form controls the easiest way to fix this seems to apply the animation only to elements having focus.

I'm able to get this working for the radio, and it works fine for the checkbox without initializing material js as you can see in this testcase.

But as soon as we initialize the JavaScript ($.material.init();), the browser stops applying css related to the :focus state of input when we click the labels. It'll start working if we activate the :focus state of input once via developer tools. This seems very strange since the JavaScript doesn't have to do anything with the checkbox animation as far as I know.

What could be the JavaScript code that is messing with the normal behavior of browser, any ideas?

Well, the culprit was this event handler:

$(document).on("change", ".checkbox input[type=checkbox]", function () {
     $(this).blur();
 })