[MWPW-170376] Preflight assets tab updates #4510
Conversation
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Additional test URLs:
|
overmyheadandbody
force-pushed
the
preflight-assets-updates
branch
2 times, most recently
from
ab6cfa9 to
606893c
Compare
overmyheadandbody
force-pushed
the
preflight-assets-updates
branch
from
606893c to
95da523
Compare
| @@ -22,110 +212,53 @@ export async function checkImageDimensions(url, area) { | |||
| return JSON.parse(JSON.stringify(cachedResult)); | |||
| } | |||
|
|
|||
| const allImages = [...area.querySelectorAll('main picture img')]; | |||
| if (!allImages.length) { | |||
| const allAssets = [...area.querySelectorAll('main picture img, main video, main .adobetv')]; | |||
There was a problem hiding this comment.
I see the video from modal is not picked up - https://preflight-assets-updates--milo--overmyheadandbody.aem.page/docs/authoring/tall-video-modal#tvid
There was a problem hiding this comment.
Good point, I'll look into this for a follow-up contribution.
| const isFullWidthAsset = offsetWidth >= viewportWidth; | ||
|
|
||
| // Define the ideal factor depending on the asset's display width | ||
| let idealFactor = isFullWidthAsset ? 1.5 : 2; |
There was a problem hiding this comment.
nit: should define the values as constants (e.g. IDEAL_IMAGE_FACTOR, IDEAL_FULL_WIDTH_IMAGE_FACTOR)
There was a problem hiding this comment.
My reason for not doing this is simply because the values are never re-used. Using constants would add a few more bytes to the size and use (a bit of) additional memory, with no obvious benefit. It's of course negligible, especially since this isn't customer-facing, so both approaches should be acceptable.
There was a problem hiding this comment.
Testing details
https://jira.corp.adobe.com/browse/MWPW-170376
This adapts the Assets checks from the Preflight suite to also check videos, not only images. To keep a god balance between performance and quality, a factor of
1has been chosen for video assets; meaning a video's dimensions should be at least those of its container. It considers:videotags. These are marked as Video (Sharepoint);iframetags. These are marked as Video (MPC);videotags. These are marked as Video (MPC source).A couple of additional checks can populate a new Notes field, as suggested here, should one of these conditions be met:
The Assets logic had to be refactored, as the logic got more complex with adding video checks. Another small change was made to the video decoration logic to ensure a source is only added if one doesn't already exist. This is needed, since the Preflight logic forces video load, even when outside the viewport, in order to perform the relevant size calculations. When a user eventually scrolls the page, we need to ensure we don't duplicate the video's
sourcechild.Resolves: MWPW-170376
Test URLs: