Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Animate remotely loaded banners together #1808
Animate remotely loaded banners together #1808
Changes from 7 commits
f1344c7
8cb830f
b048897
ca95ef6
d663d11
5fc1df3
ae52d97
613527b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I discovered this line I was a bit surprised. An
await
in the middle of a module file causes all the rest of the code below it to be deferred until the promise is settled. So this represents a pretty serious slowdown in executing this file, since pretty much all of the code in this file is actually run at the end of the file.This was introduced in #1344. Previously, the fetch was also executed at the module level, but without the
await
syntax, so it didn't hold back the rest of the file from executing.So I decided to put all of this code into a new asynchronous function,
fetchAndUseVersions
which is called when the document is ready.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function definition should arguably moved to a different part of the file, next to other code that has to do with the version switcher, but I didn't want to make the diff harder to compare so I left this code at the same place in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorporates #1755
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the error was not being printed to the console. I thought it might be helpful to our users to print the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the transition and overflow rules to the new container element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class was added by #1693. It's not needed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functionality—not showing the banner if it's empty—is now taken care of with the changes from #1703 and with the
d-none
class used in this PR.