[bug] centerHorizontalOnly plus an animated button caused showDialog promise never resolved.

[3cp]

I'm submitting a bug report

• Library Version:
  1.0.0-beta.3.0.1

Please tell us about your environment:

• Operating System:
  OSX 10.x

• Browser:
  Chrome Latest| Firefox Latest | Safari Latest

Chrome and Firefox hit the bug every time.
Safari is less affected, with a small chance to hit the bug.

• Language:
  ESNext

Current behavior:
on top of position bug #190 , an button with transition css caused showDialog promise never returned.

Expected/desired behavior:
See the example here https://github.com/huochunpeng/dialog-bug
It's an aurelia-cli project.
BTW, please provide a public bundle of aurelia including aurelia-dialog for easy gist creating.

• What is the expected behavior?
  Just make it work.

• What is the motivation / use case for changing the behavior?

[d-kostov-dev]

I can confirm this. I don't have animated button, but setting centerHorizontalOnly to true, then the dialog is never resolved.

[d-kostov-dev]

I went trough the code and the only difference is that this function is not called:

function centerDialog(modalContainer) {
  const child = modalContainer.children[0];
  const vh = Math.max(DOM.querySelectorAll('html')[0].clientHeight, window.innerHeight || 0);

  child.style.marginTop = Math.max((vh - child.offsetHeight) / 2, 30) + 'px';
  child.style.marginBottom = Math.max((vh - child.offsetHeight) / 2, 30) + 'px';
}

So I configured a position function: config.settings.position... that uses the exact same function. But when you remove this line (ofc not using 'vh' in the code after):

const vh = Math.max(DOM.querySelectorAll('html')[0].clientHeight, window.innerHeight || 0);

the dialog is not resolved. I just can't understand why.

[gama410]

I confirm I have the same problem. setting centerHorizontalOnly to true makes the promise never resolve.
I noticed that the promise resolved when I clicked on the overlay (when settings.lock = false) but never when I call controller.ok() or controller.cancel(), though...

[gama410]

While trying to bypass this problem, I tried to replace the 'centerHorizontalOnly' setting with a 'position' callback setting. This triggered the same problem...

By the way, I need to override the vertical centering because I'm displaying a dialog with a compose element inside that is populated from a server query. This means (as far as I understand it) that the dialog content is empty when the dialog's position is calculated so it is position with its top at the center of the screen and when populated, the dialog extends beyond the bottom of the screen.
I don't see this as a bug because I think I understand the behaviour and it seems correct to me but if you guys have an idea of how to handle this (without using the problematic settings), I would be very interested...

[StrahilKazlachev]

@gama410 Did you try setting ignoreTransitions: false? If that helps as a workaround, the fix for this will be in the next release.

[gama410]

@StrahilKazlachev as the default value of this settings is 'false', I guess you meant to set it to true?
When I set it to true, it indeed fixed the problem for me! Thanks a lot!

[3cp]

Boostrap doesn't use native transitionend event, but rather simulated it with setTimeout for reliable callback.

from https://github.com/twbs/bootstrap/blob/master/js/transition.js#L35-L43

  // http://blog.alexmaccaw.com/css-transitions
  $.fn.emulateTransitionEnd = function (duration) {
    var called = false
    var $el = this
    $(this).one('bsTransitionEnd', function () { called = true })
    var callback = function () { if (!called) $($el).trigger($.support.transition.end) }
    setTimeout(callback, duration)
    return this
  }

Quote from the linked article by ALEX MACCAW:

Be aware that sometimes this event doesn’t fire, usually in the case when properties don’t change or a paint isn’t triggered. To ensure we always get a callback, let’s set a timeout that’ll trigger the event manually.

@StrahilKazlachev @PWKad how do you think about

      if (ignore || !hasTransition(this.dialogContainer)) {
        resolve();
      } else {
        // this.dialogContainer.addEventListener(eventName, onTransitionEnd);
        setTimeout(resolve, TRANSITION_DURATION); // 200ms
      }

The down side is now I need to use TRANSITION_DURATION to build defaultCSSText.
I can make a PR around this.

[3cp]

huochunpeng unassigned PWKad 3 minutes ago

How did this happen?? I don't think I got the privilege for doing that.

[StrahilKazlachev]

I'll refrain from promising specific dates, but soon I'll merge the Animator support - no transitions there. I have a bit more to refactor.

[3cp]

Great to hear.