Measure Media Weight with Performance API#12
Measure Media Weight with Performance API#12ajvillegas wants to merge 7 commits intofix-editor-unexpected-errorfrom
Conversation
roborourke
left a comment
There was a problem hiding this comment.
Good stuff @ajvillegas, this is basically what I'd done / started. There's a couple of nuances to the getEntries data we could talk through when you get to that but take a look and if you want to discuss approaches let me know
kadamwhite
left a comment
There was a problem hiding this comment.
Like how this is developing, seems to be on-track @ajvillegas . Notes:
- It seems like this is set up to only measure on click, which is cool; avoids unnecessary requests. But could we alter the UI until the data has been returned? (I assume this is part of what you've been working on).
- As a follow-up, I wonder if we can integrate this into the UI more snazzily. Inject the prompt into the sidebar on any image block, for example (this would be a after-this-version-is-working idea); or tie it properly into the prepublish checks
- Once this is done-done (and not before), it'd be cool to follow up with another PR to break apart HMMediaWeightSidebar a bit
| * | ||
| * @return void | ||
| */ | ||
| function get_performance_entries() { |
There was a problem hiding this comment.
Nitpick, we usually use get_ in WP core for a function which returns things; since this outputs, I'd name it render_, output_ etc.
There was a problem hiding this comment.
Good point, I updated the function name to output_...
| const listener = window.addEventListener( 'message', ( event ) => { | ||
| const receivedEntries = event.data; | ||
| // eslint-disable-next-line no-console | ||
| console.log( receivedEntries ); | ||
| // You can call a useState callback here to get the data into the component. | ||
| } ); | ||
|
|
||
| return () => { | ||
| window.removeEventListener( listener ); | ||
| }; |
There was a problem hiding this comment.
This causes an error, removeEventListener needs the type specified; and addEventListener does not return anything, so this isn't doing what you'd expect. You'd want something more like this:
const listener = ( event ) => { /* ... */ };
window.addEventListener( 'message', listener );
return () => { window.removeEventListener( 'message', listener ); };that way you're unbinding the listener by reference.
| const listener = window.addEventListener( 'message', ( event ) => { | |
| const receivedEntries = event.data; | |
| // eslint-disable-next-line no-console | |
| console.log( receivedEntries ); | |
| // You can call a useState callback here to get the data into the component. | |
| } ); | |
| return () => { | |
| window.removeEventListener( listener ); | |
| }; | |
| const listener = ( event ) => { | |
| const receivedEntries = event.data; | |
| // eslint-disable-next-line no-console | |
| console.log( receivedEntries ); | |
| // You can call a useState callback here to get the data into the component. | |
| }; | |
| window.addEventListener( 'message', listener ); | |
| return () => { | |
| window.removeEventListener( listener ); | |
| }; |
☝️ Untested but should work
There was a problem hiding this comment.
It looks like it should work, but removeEventListener needs the event name.
window.removeEventListener( 'message', listener );
![]()
There was a problem hiding this comment.
I updated the listener as suggested.
|
@kadamwhite @roborourke I updated the PR with your suggestions and now the listener outputs a I agree that once this is ironed out, we should break up HMMediaWeightSidebar. |
This change builds off of PR #11 and introduces a way to pull asset data from the front-end using the browser's Performance API within a post preview URL iframe.
This allows for the media sizes to reflect what is being served in the front-end and can be used to calculate media weight at different screen sizes.
The draft PR is a WIP and a proof-of-concept based on @roborourke's initial research and pseudo code.