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

New times display before the updated notification #62

Open
belenbarrospena opened this issue Dec 19, 2018 · 3 comments
Open

New times display before the updated notification #62

belenbarrospena opened this issue Dec 19, 2018 · 3 comments

Comments

@belenbarrospena
Copy link
Contributor

Steps to reproduce:

  1. Load the video review page
  2. Select "The video has problems."
  3. In the "Start time" section, select "The video starts too early"
  4. Move the video scrubber forward and click the "Set new start up" button. The notification showing the new video times will fade in.
  5. Move the video scrubber forward again and click the "Set new start up" button.

Outcome: the video times will update, then the old notification will fade out, and the new one will fade in. The overall effect is a bit strange.

The correct sequence of events should be:

  1. Fade out the notification
  2. Update the video times
  3. Fade in the notification
@yoe yoe added the design label Dec 19, 2018
@yoe
Copy link
Owner

yoe commented Dec 19, 2018

We actually do it in that order, but jQuery doesn't wait. I'll have to figure out how to make jQuery wait for the previous animation before moving on to the next line of code. That'll probably be a callback or some such, I guess.

@yoe yoe added this to the FOSDEM 2019 milestone Jan 2, 2019
@belenbarrospena
Copy link
Contributor Author

Could the delay() function help with this? https://api.jquery.com/delay/

@yoe
Copy link
Owner

yoe commented Feb 5, 2019

There is actually a Promise which would help (it's a method that gets called once the fade has finished), but it gets called for each element that's being faded, so have to filter that out first. Should be doable, but could be slightly complicated

@yoe yoe added the bug label Feb 18, 2019
@yoe yoe removed this from the FOSDEM 2019 milestone Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants