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

[CLOSED] Fix unit tests broken by modal bar animation #4437

Open
core-ai-bot opened this issue Aug 29, 2021 · 5 comments
Open

[CLOSED] Fix unit tests broken by modal bar animation #4437

core-ai-bot opened this issue Aug 29, 2021 · 5 comments

Comments

@core-ai-bot
Copy link
Member

Issue by njx
Friday Aug 16, 2013 at 22:41 GMT
Originally opened as adobe/brackets#4800


The modal bar animation added by@larz0 added a bit of asynchronicity to the modal bar: it now isn't removed from the DOM until the close animation completes. This pull request fixes up the unit tests so that they wait for the previous modal bar to close (and forcibly remove it at the end of the test) before continuing.

Also, some unit tests were breaking because the modal bar was never removing itself from the DOM. This was due to a subtle issue: the new modal bar code was using the .hide class, but that class is also defined by Bootstrap to set display: none. That wasn't affecting the modal bar's behavior in Brackets itself, but was affecting it in the test runner, possibly because stylesheets are loaded in a different order. So I just renamed the class to .modal-bar-hide to avoid getting contaminated by the Bootstrap rule.


njx included the following code: https://github.com/adobe/brackets/pull/4800/commits

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Aug 16, 2013 at 22:44 GMT


I highly recommend reviewing with ?w=1 :)

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Aug 16, 2013 at 22:54 GMT


@gruehle or@peterflynn--interested in taking this one? Want to get it checked in today since the unit tests are broken.

Note that this does mean that the timing of when modal bars appear/disappear has changed...but I'm inclined not to worry about it too much unless we find actual user-visible issues.

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Aug 16, 2013 at 22:54 GMT


Oops, retagging@peterflynn

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Aug 16, 2013 at 22:57 GMT


For #4797

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Friday Aug 16, 2013 at 23:52 GMT


Looks good. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant