-
Notifications
You must be signed in to change notification settings - Fork 99
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
Leverage URL metrics to reserve space for embeds to reduce CLS #1373
Merged
westonruter
merged 59 commits into
trunk
from
add/embed-optimizer-min-height-reservation
Oct 14, 2024
Merged
Changes from all commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
0a376c1
Introduce methods to get minumum height of element
westonruter 8aa7e63
Set the min-height of an embed prior to it loading
westonruter 11f98f4
Set min-height on embed-wrapper instead of figure container
westonruter d7cc7cf
Use 500px as a better representation of an element that could be LCP
westonruter 48e57e8
Add test for existing style manipulation
westonruter 0d285f2
Add helper generator method to get all elements
westonruter 4778a3d
Try using MutationObserve to watch for embed height changes
westonruter 1dbd4a1
Use the more appropriate ResizeObserver instead of MutationObserver
westonruter a4bab7e
Remove condition that breaks monitoring resizes of post embeds
westonruter 5f0cdbe
Introduce client-side Optimization Detective extensions and move Embe…
westonruter 0ba2d6e
Override clientBoundingRect once embed has loaded
westonruter 5f4189d
Move jsdoc types to types.d.ts for reuse
westonruter bf2b3c5
Send URL metric when leaving the page
westonruter b9bad0d
Use boundingClientRect instead of intersectionRect in get_all_element…
westonruter c6b02ec
Eliminate timeout for disconneccting ResizeObsever
westonruter 52a2260
Move extension initialization after idle callback
westonruter edc52fa
Fix warning when prematurely applying buffered text replacements, esp…
westonruter 820d66d
Prepend min-height to style attribute instead of appending
westonruter def2aab
Use object spread
westonruter cd9a618
Merge branch 'trunk' of https://github.com/WordPress/performance into…
westonruter 02c4fd9
Merge branch 'trunk' into add/embed-optimizer-min-height-reservation
westonruter 1da219f
Use get_json_params() instead of get_params() so _wpnonce query param…
westonruter e34d9fe
Implement resizedBoundingClientRect extended property in schema
westonruter 5db6f54
Fix testing JSON request
westonruter 0fa263a
Go back to get_params() by ignoring _wpnonce
westonruter 72b285d
Merge branch 'trunk' into add/embed-optimizer-min-height-reservation
westonruter 2a723f7
Fix jsdoc
westonruter 71dd914
Merge branch 'trunk' of https://github.com/WordPress/performance into…
westonruter 29d4383
Eliminate use of deprecated property
westonruter a529218
Add breakpoint-specific min-heights to account for responsive embeds
westonruter fa8a34e
Add od_generate_media_query() helper
westonruter 1e40f84
Break up embed tag visitor into separate methods
westonruter 5d4d5b2
Bump alpha versions
westonruter 5f1c2ac
Add missing short-circuit in case EMBED_OPTIMIZER_VERSION is defined
westonruter 915e1e7
Rework bootstrap logic to wait until init priority 9 and add od_init …
westonruter 26ae396
Add test for when resizedBoundingClientRect data not available
westonruter cd80ed1
Remove obsolete short-circuiting now that OD dependency version is ch…
westonruter bd008c5
Evolve get_all_url_metrics_groups_elements into get_all_denormalized_…
westonruter 1b5cf13
Add Embed Optimizer tests
westonruter a70df28
Account for error when passing single-item array to min() or max()
westonruter 4e48d3d
Add test for Image Prioritizer
westonruter d17cace
Remove now-unused method to get element minimum hights
westonruter 5574081
Improve handling of get_updated_html
westonruter 19c0425
Add test for get_all_denormalized_elements
westonruter ea36bac
Add tests for new OD code
westonruter 01c083d
Clarify purpose of overridden get_updated_html method
westonruter c5d6991
Add missing since tags
westonruter 455ef4f
Clarify handling of embed block tags and embed wrapper tags
westonruter a390e15
Replace tuple with assoc array
westonruter a760705
Add doc block for detect.js
westonruter 7ca1fbc
Add API functions to pass to finalize callbacks to avoid direct mutat…
westonruter f66445f
Improve error handling
westonruter 6e0aa8e
Harden types and disallow setting core properties
westonruter 9e99e0d
Reuse sets for reserved property keys
westonruter 46ba7e3
Move functions to root of module
westonruter 0bc521e
Fix TypeScript error related to embedWrapper
westonruter 477cc33
Add missing period to return doc
westonruter dca43e9
Eliminate needless ternary
westonruter 73d2252
Rename amend to extend
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
/** | ||
* Embed Optimizer module for Optimization Detective | ||
* | ||
* When a URL metric is being collected by Optimization Detective, this module adds a ResizeObserver to keep track of | ||
* the changed heights for embed blocks. This data is extended/amended onto the element data of the pending URL metric | ||
* when it is submitted for storage. | ||
*/ | ||
|
||
const consoleLogPrefix = '[Embed Optimizer]'; | ||
felixarntz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* @typedef {import("../optimization-detective/types.d.ts").ElementData} ElementMetrics | ||
* @typedef {import("../optimization-detective/types.d.ts").URLMetric} URLMetric | ||
* @typedef {import("../optimization-detective/types.d.ts").Extension} Extension | ||
* @typedef {import("../optimization-detective/types.d.ts").InitializeCallback} InitializeCallback | ||
* @typedef {import("../optimization-detective/types.d.ts").InitializeArgs} InitializeArgs | ||
* @typedef {import("../optimization-detective/types.d.ts").FinalizeArgs} FinalizeArgs | ||
* @typedef {import("../optimization-detective/types.d.ts").FinalizeCallback} FinalizeCallback | ||
* @typedef {import("../optimization-detective/types.d.ts").ExtendedElementData} ExtendedElementData | ||
*/ | ||
|
||
/** | ||
* Logs a message. | ||
* | ||
* @param {...*} message | ||
*/ | ||
function log( ...message ) { | ||
// eslint-disable-next-line no-console | ||
console.log( consoleLogPrefix, ...message ); | ||
} | ||
|
||
/** | ||
* Logs an error. | ||
* | ||
* @param {...*} message | ||
*/ | ||
function error( ...message ) { | ||
// eslint-disable-next-line no-console | ||
console.error( consoleLogPrefix, ...message ); | ||
} | ||
|
||
/** | ||
* Embed element heights. | ||
* | ||
* @type {Map<string, DOMRectReadOnly>} | ||
*/ | ||
const loadedElementContentRects = new Map(); | ||
|
||
/** | ||
* Initializes extension. | ||
* | ||
* @type {InitializeCallback} | ||
* @param {InitializeArgs} args Args. | ||
*/ | ||
export function initialize( { isDebug } ) { | ||
/** @type NodeListOf<HTMLDivElement> */ | ||
const embedWrappers = document.querySelectorAll( | ||
'.wp-block-embed > .wp-block-embed__wrapper[data-od-xpath]' | ||
); | ||
|
||
for ( const embedWrapper of embedWrappers ) { | ||
monitorEmbedWrapperForResizes( embedWrapper, isDebug ); | ||
} | ||
|
||
if ( isDebug ) { | ||
log( 'Loaded embed content rects:', loadedElementContentRects ); | ||
} | ||
} | ||
|
||
/** | ||
* Finalizes extension. | ||
* | ||
* @type {FinalizeCallback} | ||
* @param {FinalizeArgs} args Args. | ||
*/ | ||
export async function finalize( { | ||
isDebug, | ||
getElementData, | ||
extendElementData, | ||
} ) { | ||
for ( const [ xpath, domRect ] of loadedElementContentRects.entries() ) { | ||
try { | ||
extendElementData( xpath, { | ||
resizedBoundingClientRect: domRect, | ||
} ); | ||
if ( isDebug ) { | ||
const elementData = getElementData( xpath ); | ||
log( | ||
`boundingClientRect for ${ xpath } resized:`, | ||
elementData.boundingClientRect, | ||
'=>', | ||
domRect | ||
); | ||
} | ||
} catch ( err ) { | ||
error( | ||
`Failed to extend element data for ${ xpath } with resizedBoundingClientRect:`, | ||
domRect, | ||
err | ||
); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Monitors embed wrapper for resizes. | ||
* | ||
* @param {HTMLDivElement} embedWrapper Embed wrapper DIV. | ||
* @param {boolean} isDebug Whether debug. | ||
*/ | ||
function monitorEmbedWrapperForResizes( embedWrapper, isDebug ) { | ||
if ( ! ( 'odXpath' in embedWrapper.dataset ) ) { | ||
throw new Error( 'Embed wrapper missing data-od-xpath attribute.' ); | ||
} | ||
const xpath = embedWrapper.dataset.odXpath; | ||
const observer = new ResizeObserver( ( entries ) => { | ||
const [ entry ] = entries; | ||
loadedElementContentRects.set( xpath, entry.contentRect ); | ||
if ( isDebug ) { | ||
log( `Resized element ${ xpath }:`, entry.contentRect ); | ||
} | ||
} ); | ||
observer.observe( embedWrapper, { box: 'content-box' } ); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
See note below on the
get_all_denormalized_elements()
method that I'd like to instead have$element
be anOD_Element
class instance which has a$url_metric
property to access theOD_URL_Metric
instance if is a part of. In the same way, theOD_URL_Metric
class can have a$group
property to indicate theOD_URL_Metric_Group
it is a part of. This would eliminate the need to pass back an array of tuples and instead. So instead, to get the$group
this code could do$element->url_metric->group
.This I want to do in a follow-up PR.
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.
See #1585