-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add 'defer' attribute to SDK Javascript assets. #473
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR adds the 'defer' attribute to the `script` tags for answers.min.js and answerstemplates.compiled.min.js. This will ensure that the loading of these JS assets does not block the browser from creating the DOM. This attribute, as opposed to `async`, allows us to define an ordering between the `script`s. The templates must be loaded before the SDK itself. Note that the 'async' attribute was not added to these `script`s. This is because they need to be executed in the order mentioned above, after the DOM is built. Further, having both 'async' and 'defer' allows a browser that does not support 'async' to fallback to 'defer'. We just always want 'defer'. J=SLAP-860 TEST=manual Built a local site with these changes. Did some smoke testing and saw no problems.
alexisgrow
approved these changes
Nov 16, 2020
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 lgtm!
nanhu95
approved these changes
Nov 16, 2020
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.
LGTM!
tmeyer2115
added a commit
that referenced
this pull request
Nov 16, 2020
This PR adds the 'defer' attribute to the `script` tags for answers.min.js and answerstemplates.compiled.min.js. This will ensure that the loading of these JS assets does not block the browser from creating the DOM. This attribute, as opposed to `async`, allows us to define an ordering between the `script`s. The templates must be loaded before the SDK itself. Note that the 'async' attribute was not added to these `script`s. This is because they need to be executed in the order mentioned above, after the DOM is built. Further, having both 'async' and 'defer' allows a browser that does not support 'async' to fallback to 'defer'. We just always want 'defer'. J=SLAP-860 TEST=manual Built a local site with these changes. Did some smoke testing and saw no problems.
Merged
tmeyer2115
added a commit
that referenced
this pull request
Dec 1, 2020
## Version 1.17.0 Version 1.17.0 of the Theme supports the new Overlay integration. It also has a number of changes to support the new, built-in Product entity type. ### Changes - A new file, static/js/formatters-custom.js was added to the Theme. This file is where any custom formatters or overrides of built-in formatters, should live. (#448) - Single language cards now use the `toLocalizedDistance` formatter to properly format entity distance. (#449) - To better support WCAG, the iFrame integration now has a `title` attribute on the iFrame. (#451) - The Theme now allows a new type of Answers integration: the Overlay. This is one of the simplest integrations offered. Additional documentation on it can be found in the Hitchhiker training. - We upgraded the Theme to use Webpack v5. This will enable proper tree-shaking of bundle.js. (#463, #470) - A new card type was added: `product-prominentimage-clickable`. The card itself is clickable. (#466) - A new formatter was added: `Formatter.hoursList`. This formatter lists the hours for an entity for every day of the week. (#467) - To improve site performance, the `defer` attribute was added to all the SDK JS assets. (#473, #474) - Small updates were made to all the product cards to support the new, built-in Product entity type. (#411, #476) - A new formatter was added for prices: `Formatter.price`. (#472) - The `JAMBO_INJECTED_DATA` environment variable now includes the label of an experience's vertical. This label is used as the default vertical name in navigation, no results, and universal section title. (#490, 493) ### Bug Fixes - The post upgrade script now works correctly when the Theme has been imported as a submodule. (#441) - The `LocationStandard` card's styling was updated to be fully compatible with IE11. (#455) - A bug was fixed that caused the Theme's inline Javascript to be minified twice. By removing this bug, Webpack builds were made considerably more performant. (#461) - Fixes were made to ensure the `View Results` button for `CollapsibleFilters` is sticky. (#468, #469) - The logic for identifying absolute URLs in the HBS templates was corrected (#475) - The thumbnail selection logic in `Formatter.image` was incorrect. It was not properly selecting the smallest thumbnail greater than the provided threshold. This has been corrected. Note that this bug had an impact on page performance as unnecessarily large images were loaded on the page. (#471) - The `relativePath` computed by Jambo is now properly passed along to the Theme's cards. (#499)
Merged
tmeyer2115
added a commit
that referenced
this pull request
Dec 1, 2020
## Version 1.17.0 Version 1.17.0 of the Theme supports the new Overlay integration. It also has a number of changes to support the new, built-in Product entity type. ### Changes - A new file, static/js/formatters-custom.js was added to the Theme. This file is where any custom formatters or overrides of built-in formatters, should live. (#448) - Single language cards now use the `toLocalizedDistance` formatter to properly format entity distance. (#449) - To better support WCAG, the iFrame integration now has a `title` attribute on the iFrame. (#451) - The Theme now allows a new type of Answers integration: the Overlay. This is one of the simplest integrations offered. Additional documentation on it can be found in the Hitchhiker training. - We upgraded the Theme to use Webpack v5. This will enable proper tree-shaking of bundle.js. (#463, #470) - A new card type was added: `product-prominentimage-clickable`. The card itself is clickable. (#466) - A new formatter was added: `Formatter.hoursList`. This formatter lists the hours for an entity for every day of the week. (#467) - To improve site performance, the `defer` attribute was added to all the SDK JS assets. (#473, #474) - Small updates were made to all the product cards to support the new, built-in Product entity type. (#411, #476) - A new formatter was added for prices: `Formatter.price`. (#472) - The `JAMBO_INJECTED_DATA` environment variable now includes the label of an experience's vertical. This label is used as the default vertical name in navigation, no results, and universal section title. (#490, 493) ### Bug Fixes - The post upgrade script now works correctly when the Theme has been imported as a submodule. (#441) - The `LocationStandard` card's styling was updated to be fully compatible with IE11. (#455) - A bug was fixed that caused the Theme's inline Javascript to be minified twice. By removing this bug, Webpack builds were made considerably more performant. (#461) - Fixes were made to ensure the `View Results` button for `CollapsibleFilters` is sticky. (#468, #469) - The logic for identifying absolute URLs in the HBS templates was corrected (#475) - The thumbnail selection logic in `Formatter.image` was incorrect. It was not properly selecting the smallest thumbnail greater than the provided threshold. This has been corrected. Note that this bug had an impact on page performance as unnecessarily large images were loaded on the page. (#471) - The `relativePath` computed by Jambo is now properly passed along to the Theme's cards. (#499)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds the 'defer' attribute to the
script
tags for answers.min.jsand answerstemplates.compiled.min.js. This will ensure that the loading of
these JS assets does not block the browser from creating the DOM. This
attribute, as opposed to
async
, allows us to define an ordering betweenthe
script
s. The templates must be loaded before the SDK itself.Note that the 'async' attribute was not added to these
script
s. This isbecause they need to be executed in the order mentioned above, after the DOM
is built. Further, having both 'async' and 'defer' allows a browser that does
not support 'async' to fallback to 'defer'. We just always want 'defer'.
J=SLAP-860
TEST=manual
Built a local site with these changes. Did some smoke testing and saw no
problems.