Skip to content

Variable transition durations #25662

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.

Sign up for GitHub

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

Merged
Johann-S merged 5 commits into from
Mar 13, 2018
Merged

Variable transition durations #25662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b06f41c
Variable transition durations
MartijnCuppens Feb 19, 2018
81bd16c
Merge branch 'v4-dev' into variable-transition-durations
MartijnCuppens Mar 13, 2018
be57118
More explicit detection of ms unit
MartijnCuppens Mar 13, 2018
5c0b61f
Merge branch 'v4-dev' into variable-transition-durations
XhmikosR Mar 13, 2018
ee6a836
Remove unneeded check for ms, jQuery takes care of this
MartijnCuppens Mar 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/4.0/components/carousel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,4 +327,4 @@ $('#myCarousel').on('slide.bs.carousel', function () {

### Change transition duration

The transition duration of `.carousel-item` can be changed with the `$carousel-transition` Sass variable before compiling or custom styles if you're using the compiled CSS. If multiple transitions are applied, make sure the transform transition is defined first (eg. `transition: transform 2s ease, opacity .5s ease-out`). The transition duration must be the same for each carousel item.
The transition duration of `.carousel-item` can be changed with the `$carousel-transition` Sass variable before compiling or custom styles if you're using the compiled CSS. If multiple transitions are applied, make sure the transform transition is defined first (eg. `transition: transform 2s ease, opacity .5s ease-out`).
5 changes: 3 additions & 2 deletions js/src/alert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const Alert = (($) => {
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 150

const Selector = {
DISMISS : '[data-dismiss="alert"]'
Expand Down Expand Up @@ -109,9 +108,11 @@ const Alert = (($) => {
return
}

const transitionDuration = Util.getTransitionDurationFromElement(element)

$(element)
.one(Util.TRANSITION_END, (event) => this._destroyElement(element, event))
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
}

_destroyElement(element) {
Expand Down
43 changes: 12 additions & 31 deletions js/src/carousel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,15 @@ const Carousel = (($) => {
* ------------------------------------------------------------------------
*/

const NAME = 'carousel'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.carousel'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const ARROW_LEFT_KEYCODE = 37 // KeyboardEvent.which value for left arrow key
const ARROW_RIGHT_KEYCODE = 39 // KeyboardEvent.which value for right arrow key
const TOUCHEVENT_COMPAT_WAIT = 500 // Time for mouse compat events to fire after touch
const MILLISECONDS_MULTIPLIER = 1000
const NAME = 'carousel'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.carousel'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const ARROW_LEFT_KEYCODE = 37 // KeyboardEvent.which value for left arrow key
const ARROW_RIGHT_KEYCODE = 39 // KeyboardEvent.which value for right arrow key
const TOUCHEVENT_COMPAT_WAIT = 500 // Time for mouse compat events to fire after touch

const Default = {
interval : 5000,
Expand Down Expand Up @@ -102,8 +101,6 @@ const Carousel = (($) => {
this._element = $(element)[0]
this._indicatorsElement = $(this._element).find(Selector.INDICATORS)[0]

this._transitionDuration = this._getTransitionDuration()

this._addEventListeners()
}

Expand Down Expand Up @@ -225,24 +222,6 @@ const Carousel = (($) => {
return config
}

_getTransitionDuration() {
// Get transition-duration of first element in the carousel
let transitionDuration = $(this._element).find(Selector.ITEM).css('transition-duration')

// Return 0 carousel item is not found
if (!transitionDuration) {
return 0
}

// If multiple durations are defined, take the first
transitionDuration = transitionDuration.split(',')[0]

// Multiply by 1000 if transition-duration is defined in seconds
return transitionDuration.indexOf('ms') > -1
? parseFloat(transitionDuration)
: parseFloat(transitionDuration) * MILLISECONDS_MULTIPLIER
}

_addEventListeners() {
if (this._config.keyboard) {
$(this._element)
Expand Down Expand Up @@ -406,6 +385,8 @@ const Carousel = (($) => {
$(activeElement).addClass(directionalClassName)
$(nextElement).addClass(directionalClassName)

const transitionDuration = Util.getTransitionDurationFromElement(activeElement)

$(activeElement)
.one(Util.TRANSITION_END, () => {
$(nextElement)
Expand All @@ -418,7 +399,7 @@ const Carousel = (($) => {

setTimeout(() => $(this._element).trigger(slidEvent), 0)
})
.emulateTransitionEnd(this._transitionDuration)
.emulateTransitionEnd(transitionDuration)
} else {
$(activeElement).removeClass(ClassName.ACTIVE)
$(nextElement).addClass(ClassName.ACTIVE)
Expand Down
8 changes: 5 additions & 3 deletions js/src/collapse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const Collapse = (($) => {
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 600

const Default = {
toggle : true,
Expand Down Expand Up @@ -190,10 +189,11 @@ const Collapse = (($) => {

const capitalizedDimension = dimension[0].toUpperCase() + dimension.slice(1)
const scrollSize = `scroll${capitalizedDimension}`
const transitionDuration = Util.getTransitionDurationFromElement(this._element)

$(this._element)
.one(Util.TRANSITION_END, complete)
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)

this._element.style[dimension] = `${this._element[scrollSize]}px`
}
Expand Down Expand Up @@ -252,9 +252,11 @@ const Collapse = (($) => {
return
}

const transitionDuration = Util.getTransitionDurationFromElement(this._element)

$(this._element)
.one(Util.TRANSITION_END, complete)
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
}

setTransitioning(isTransitioning) {
Expand Down
33 changes: 20 additions & 13 deletions js/src/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ const Modal = (($) => {
* ------------------------------------------------------------------------
*/

const NAME = 'modal'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.modal'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 300
const BACKDROP_TRANSITION_DURATION = 150
const ESCAPE_KEYCODE = 27 // KeyboardEvent.which value for Escape (Esc) key
const NAME = 'modal'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.modal'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const ESCAPE_KEYCODE = 27 // KeyboardEvent.which value for Escape (Esc) key

const Default = {
backdrop : true,
Expand Down Expand Up @@ -187,10 +185,13 @@ const Modal = (($) => {
$(this._element).off(Event.CLICK_DISMISS)
$(this._dialog).off(Event.MOUSEDOWN_DISMISS)


if (transition) {
const transitionDuration = Util.getTransitionDurationFromElement(this._element)

$(this._element)
.one(Util.TRANSITION_END, (event) => this._hideModal(event))
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
} else {
this._hideModal()
}
Expand Down Expand Up @@ -263,9 +264,11 @@ const Modal = (($) => {
}

if (transition) {
const transitionDuration = Util.getTransitionDurationFromElement(this._element)

$(this._dialog)
.one(Util.TRANSITION_END, transitionComplete)
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
} else {
transitionComplete()
}
Expand Down Expand Up @@ -369,9 +372,11 @@ const Modal = (($) => {
return
}

const backdropTransitionDuration = Util.getTransitionDurationFromElement(this._backdrop)

$(this._backdrop)
.one(Util.TRANSITION_END, callback)
.emulateTransitionEnd(BACKDROP_TRANSITION_DURATION)
.emulateTransitionEnd(backdropTransitionDuration)
} else if (!this._isShown && this._backdrop) {
$(this._backdrop).removeClass(ClassName.SHOW)

Expand All @@ -384,9 +389,11 @@ const Modal = (($) => {

if (Util.supportsTransitionEnd() &&
$(this._element).hasClass(ClassName.FADE)) {
const backdropTransitionDuration = Util.getTransitionDurationFromElement(this._backdrop)

$(this._backdrop)
.one(Util.TRANSITION_END, callbackRemove)
.emulateTransitionEnd(BACKDROP_TRANSITION_DURATION)
.emulateTransitionEnd(backdropTransitionDuration)
} else {
callbackRemove()
}
Expand Down
17 changes: 9 additions & 8 deletions js/src/tab.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ const Tab = (($) => {
* ------------------------------------------------------------------------
*/

const NAME = 'tab'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.tab'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 150
const NAME = 'tab'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.tab'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]

const Event = {
HIDE : `hide${EVENT_KEY}`,
Expand Down Expand Up @@ -162,9 +161,11 @@ const Tab = (($) => {
)

if (active && isTransitioning) {
const transitionDuration = Util.getTransitionDurationFromElement(active)

$(active)
.one(Util.TRANSITION_END, complete)
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
} else {
complete()
}
Expand Down
21 changes: 12 additions & 9 deletions js/src/tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ const Tooltip = (($) => {
* ------------------------------------------------------------------------
*/

const NAME = 'tooltip'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.tooltip'
const EVENT_KEY = `.${DATA_KEY}`
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 150
const CLASS_PREFIX = 'bs-tooltip'
const NAME = 'tooltip'
const VERSION = '4.0.0'
const DATA_KEY = 'bs.tooltip'
const EVENT_KEY = `.${DATA_KEY}`
const JQUERY_NO_CONFLICT = $.fn[NAME]
const CLASS_PREFIX = 'bs-tooltip'
const BSCLS_PREFIX_REGEX = new RegExp(`(^|\\s)${CLASS_PREFIX}\\S+`, 'g')

const DefaultType = {
Expand Down Expand Up @@ -335,9 +334,11 @@ const Tooltip = (($) => {
}

if (Util.supportsTransitionEnd() && $(this.tip).hasClass(ClassName.FADE)) {
const transitionDuration = Util.getTransitionDurationFromElement(this.tip)

$(this.tip)
.one(Util.TRANSITION_END, complete)
.emulateTransitionEnd(Tooltip._TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
} else {
complete()
}
Expand Down Expand Up @@ -384,9 +385,11 @@ const Tooltip = (($) => {

if (Util.supportsTransitionEnd() &&
$(this.tip).hasClass(ClassName.FADE)) {
const transitionDuration = Util.getTransitionDurationFromElement(tip)

$(tip)
.one(Util.TRANSITION_END, complete)
.emulateTransitionEnd(TRANSITION_DURATION)
.emulateTransitionEnd(transitionDuration)
} else {
complete()
}
Expand Down
18 changes: 18 additions & 0 deletions js/src/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const Util = (($) => {
let transition = false

const MAX_UID = 1000000
const MILLISECONDS_MULTIPLIER = 1000

// Shoutout AngusCroll (https://goo.gl/pxwQGp)
function toType(obj) {
Expand Down Expand Up @@ -104,6 +105,23 @@ const Util = (($) => {
}
},

getTransitionDurationFromElement(element) {
Copy link
Member

@Johann-S Johann-S Feb 20, 2018

Choose a reason for hiding this comment

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

Please add a unit test for this method

Copy link
Member Author

@MartijnCuppens MartijnCuppens Feb 20, 2018

Choose a reason for hiding this comment

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

See #25218 (review)

Copy link
Member

@Johann-S Johann-S Feb 20, 2018

Choose a reason for hiding this comment

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

You can test when you have multi durations and when you have ms

Copy link
Member Author

@MartijnCuppens MartijnCuppens Feb 23, 2018

Choose a reason for hiding this comment

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

Unit tests are added: 05cef0a

// Get transition-duration of the element
let transitionDuration = $(element).css('transition-duration')

// Return 0 if element or transition duration is not found
if (!transitionDuration) {
return 0
}

// If multiple durations are defined, take the first
transitionDuration = transitionDuration.split(',')[0]

// jQuery always converts transition durations into seconds,
// so multiply by 1000
return parseFloat(transitionDuration) * MILLISECONDS_MULTIPLIER
},

reflow(element) {
return element.offsetHeight
},
Expand Down
44 changes: 44 additions & 0 deletions js/tests/unit/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,50 @@ $(function () {
assert.strictEqual(typeof Util.isElement({}) === 'undefined', true)
})

QUnit.test('Util.getTransitionDurationFromElement should accept transition durations in milliseconds', function (assert) {
assert.expect(1)
var $div = $('<div style="transition: all 300ms ease-out;"></div>').appendTo($('#qunit-fixture'))
Copy link
Member

@Johann-S Johann-S Mar 12, 2018

Choose a reason for hiding this comment

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

This test do not work, I don't know how but your 300ms is converted into seconds in your JS, see our coverage :

image

Copy link
Member Author

@MartijnCuppens MartijnCuppens Mar 13, 2018

Choose a reason for hiding this comment

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

What did you do to detect this? npm test doesn't return any errors and the visual tests look fine?

Copy link
Member

@XhmikosR XhmikosR Mar 13, 2018

Choose a reason for hiding this comment

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

It's the coverage, see #25783

Copy link

@artifex404 artifex404 Mar 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

My 2 cents are, that the condition it should test should be: transitionDuration.indexOf('ms') !== -1 to be more explicit?

Copy link
Member

@XhmikosR XhmikosR Mar 13, 2018

Choose a reason for hiding this comment

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

Indeed, indexOf returns -1 if not found. So !== -1 would be better.

Copy link
Member Author

@MartijnCuppens MartijnCuppens Mar 13, 2018

Choose a reason for hiding this comment

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

Changed it to transitionDuration.indexOf('ms') !== -1, problem with coverage still exists.

Copy link
Member

@Johann-S Johann-S Mar 13, 2018

Choose a reason for hiding this comment

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

As I said on my first message, this line :

let transitionDuration = $(element).css('transition-duration')

auto convert transition-duration into seconds.

IMO jQuery use getComputedStyle which do that

Copy link
Member Author

@MartijnCuppens MartijnCuppens Mar 13, 2018

Choose a reason for hiding this comment

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

jQuery takes care of this indeed (Also tested this in Chrome, FF, IE).

IMO it's safe to remove this check as long as we depend on jQuery.

Copy link
Member

@Johann-S Johann-S Mar 13, 2018

Choose a reason for hiding this comment

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

I don't think it's jQuery which do that, IMO it's more getComputedStyle but that's OK to me 👍


assert.strictEqual(Util.getTransitionDurationFromElement($div), 300)
})

QUnit.test('Util.getTransitionDurationFromElement should accept transition durations in seconds', function (assert) {
assert.expect(1)
var $div = $('<div style="transition: all .4s ease-out;"></div>').appendTo($('#qunit-fixture'))

assert.strictEqual(Util.getTransitionDurationFromElement($div), 400)
})

QUnit.test('Util.getTransitionDurationFromElement should get the first transition duration if multiple transition durations are defined', function (assert) {
assert.expect(1)
var $div = $('<div style="transition: transform .3s ease-out, opacity .2s;"></div>').appendTo($('#qunit-fixture'))

assert.strictEqual(Util.getTransitionDurationFromElement($div), 300)
})

QUnit.test('Util.getTransitionDurationFromElement should return 0 if transition duration is not defined', function (assert) {
assert.expect(1)
var $div = $('<div></div>').appendTo($('#qunit-fixture'))

assert.strictEqual(Util.getTransitionDurationFromElement($div), 0)
})

QUnit.test('Util.getTransitionDurationFromElement should return 0 if element is not found in DOM', function (assert) {
assert.expect(1)
var $div = $('#fake-id')

assert.strictEqual(Util.getTransitionDurationFromElement($div), 0)
})

QUnit.test('Util.getTransitionDurationFromElement should properly handle inherited transition durations', function (assert) {
assert.expect(1)
var $parent = $('<div style="transition-duration: 5s;"></div>')
var $child = $('<div style="transition-duration: inherit;"></div>')
$('#qunit-fixture').append($parent.append($child))

assert.strictEqual(Util.getTransitionDurationFromElement($child), 5000)
})

QUnit.test('Util.getUID should generate a new id uniq', function (assert) {
assert.expect(2)
var id = Util.getUID('test')
Expand Down
7 changes: 7 additions & 0 deletions js/tests/visual/alert.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ <h1>Alert <small>Bootstrap Visual Test</small></h1>
<button type="button" class="btn btn-primary">Or do this</button>
</p>
</div>

<div class="alert alert-warning alert-dismissible fade show" role="alert" style="transition-duration: 5s;">
<button type="button" class="close" data-dismiss="alert" aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
This alert will take 5 seconds to fade out.
</div>
</div>

<script src="../../../assets/js/vendor/jquery-slim.min.js"></script>
Expand Down
Loading