twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
170.48k stars 78.83k forks source link

Add option to enable/disable animation of Collapse on a per-show/hide basis #18127

Closed wnr closed 5 years ago

wnr commented 8 years ago

See #15747 for the original feature request.

What we want: An additional JavaScript option of $(e).collapse() that let us disable the animation.

My proposal is that such option would be named animate, and has a default value of true.

Example usage:

// Collapses the element without any animation.
$(element).collapse({
  toggle: true,
  animate: false
});

// Collapses the element with animation, as animate is true per default.
$(element).collapse({
  toggle: true
});

There are probably other Bootstrap APIs that could also benefit by having such option.

If this sounds good, I could look into creating a PR.

cvrebert commented 8 years ago

For clarity, you mean animate should be a parameter of the .collapse('show')/.collapse('hide')/.collapse('toggle') methods, right?

wnr commented 8 years ago

Yes On Mon, 2 Nov 2015 at 10:50, Chris Rebert notifications@github.com wrote:

For clarity, you mean animate should be a parameter of the .collapse('show')/.collapse('hide')/.collapse('toggle') methods, right?

— Reply to this email directly or view it on GitHub https://github.com/twbs/bootstrap/issues/18127#issuecomment-152972631.

wnr commented 8 years ago

So do you think this is a good approach? Shall I create a PR?

cvrebert commented 8 years ago

Yeah, sure. Note that this will require adding visual tests (which are run interactively in real browsers and require manual verification by humans) since it's transition-related. Also, I'd recommend only tackling the Collapse plugin for now, so that any code style or test-related kinks can get worked out first.

wnr commented 8 years ago

Sure, sounds good.

wnr commented 8 years ago

So I just realised that I have misunderstood the API a bit. I thought that invoking $(e).collapse(config) updates the current config of the element. This seems to not be the case.

I've created a branch that is able to disable/enable animations, but it is currently not possible to change the behaviour of an already inited element. I think that a logical next step would be to read the config of an element per invocation so that changed attributes or collapse(config) calls are considered.

The other approach would be having an optional config object parameter along with the hide/show method calls, such as $(e).collapse(config, 'hide') as we previously discussed. But I feel like the first approach might be desired.

Shall I submit the PR with the animation option and open a separate issue for updating the config of an element, or shall it also be done in this PR?

cvrebert commented 8 years ago

Separate PRs please.

gqqnbig commented 8 years ago

Another optimizaiton is if the element in start position and in end position are both not in viewport, then show/hide right away, not use animation, as user won't see it.

pvdlg commented 7 years ago

This PR #21589 might be a solution for this requirement. It allows to adapt the animation duration based on the css value rather than a static value as it is currently.

Then in order to show/hide without an animation, we can define

.no-transition {
    transition: none !important;
}

Then either set the no-transition class directly in the markup for the collapse or

$('.collapse').addClass('no-transition').collapse('toggle')
ysds commented 5 years ago

Closing per #25662

demo: https://codepen.io/anon/pen/KLPMrQ

porterwilcox commented 1 year ago
.no-transition {
    transition: none !important;
    animation: none !important; /* This is required for my solution, too */
}