Tiled gallery: Remove unused view dependencies #11326
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
|
Caution: This PR has changes that must be merged to WordPress.com |
3 tasks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
With some restructuring, the dependency can be removed. Depends on: Automattic/wp-calypso#30752
|
sirreal, Your synced wpcom patch D24243-code has been updated. |
|
A fix for the |
simison
reviewed
Feb 13, 2019
simison
previously approved these changes
Feb 13, 2019
There was a problem hiding this comment.
Works well together with Automattic/wp-calypso#30752
Thanks for the cleanup!
|
sirreal, Your synced wpcom patch D24243-code has been updated. |
|
@simison care to re-approve? Calypso-side has landed so this should be easier to test. |
|
Related: D24306-code (diff dependency) |
|
If landed, this would break the master build because it depends on block changes that haven't landed in master yet. We should likely publish a new version and pull it in before landing this PR. |
|
Related discussion: p1550681303020100-slack-jetpack-gutenberg |
|
Merging this, as Jetpack now has a new version of the blocks including the Calypso part of this change. (#11420) |
This was referenced Mar 5, 2019
jeherve
pushed a commit
that referenced
this pull request
Mar 6, 2019
The previous version of Tiled gallery had `wp-i18n` as a dependency which depends on `wp-polyfill` (https://github.com/WordPress/WordPress/blob/029fcf7791e425303d14328471ed61e9f0ecc1e1/wp-includes/script-loader.php#L424). We removed view side deps in #11326 By adding `wp-polyfill`, we ensure that `isNaN`, `Array.from` and other methods that need polyfilling to work in IE11. Editor side dependency isn't _really_ needed here since many other dependencies pull in wp-polyfill as well, but seems like it's good to be explicit? This fix is an alternative to Automattic/wp-calypso#31228 because of size considerations: Automattic/wp-calypso#31228 (comment): >wp-polyfill (v7.0.0) 30.7 KB minified > >view.js when building minified production versions from Automattic/wp-calypso#31228: >- without polyfill 7.96 KiB >- with polyfill 87.6 KiB #### Changes proposed in this Pull Request: - Add `wp-polyfill` as a dependency for tiled gallery block view side and editor side. #### Testing instructions: - Spin up jurassic ninja with this branch gutenpack-jn - Add Tiled gallery to a page where no other blocks are not present (to ensure you're not loading `wp-polyfill` via other sources) - open the saved page in IE11 and observe no more fatal errors in debugger Fixes Automattic/wp-calypso#31224 #### Proposed changelog entry for your changes: Fix regression that caused Tiled Gallery not function in Internet Explorer 11 browsers anymore.
jeherve
pushed a commit
that referenced
this pull request
Mar 6, 2019
The previous version of Tiled gallery had `wp-i18n` as a dependency which depends on `wp-polyfill` (https://github.com/WordPress/WordPress/blob/029fcf7791e425303d14328471ed61e9f0ecc1e1/wp-includes/script-loader.php#L424). We removed view side deps in #11326 By adding `wp-polyfill`, we ensure that `isNaN`, `Array.from` and other methods that need polyfilling to work in IE11. Editor side dependency isn't _really_ needed here since many other dependencies pull in wp-polyfill as well, but seems like it's good to be explicit? This fix is an alternative to Automattic/wp-calypso#31228 because of size considerations: Automattic/wp-calypso#31228 (comment): >wp-polyfill (v7.0.0) 30.7 KB minified > >view.js when building minified production versions from Automattic/wp-calypso#31228: >- without polyfill 7.96 KiB >- with polyfill 87.6 KiB #### Changes proposed in this Pull Request: - Add `wp-polyfill` as a dependency for tiled gallery block view side and editor side. #### Testing instructions: - Spin up jurassic ninja with this branch gutenpack-jn - Add Tiled gallery to a page where no other blocks are not present (to ensure you're not loading `wp-polyfill` via other sources) - open the saved page in IE11 and observe no more fatal errors in debugger Fixes Automattic/wp-calypso#31224 #### Proposed changelog entry for your changes: Fix regression that caused Tiled Gallery not function in Internet Explorer 11 browsers anymore.
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.
Remove unused view dependencies from the Tiled Gallery block.
Fixes Automattic/wp-calypso#28933
Changes proposed in this Pull Request:
Testing instructions:
Test with
calypsobranch=update/g7g-tg-restructure-drop-view-i18n-deplodashwp-i18n(depends on Tiled Gallery: Move localized style labels into settings wp-calypso#30752)wp-token-listProposed changelog entry for your changes: