core(image-elements): restructure for clarity #12568
Conversation
patrickhulce
force-pushed
the
image_v8_rework
branch
from
a6c184c to
2869b7e
Compare
types/artifacts.d.ts
Outdated
| /** | ||
| * The computed style | ||
| */ | ||
| computedStyle: { |
There was a problem hiding this comment.
computedStyles ? or cssComputedStyles to match the other property?
There was a problem hiding this comment.
I was aiming to match getComputedStyle, but SGTM :)
| "height": 16 | ||
| }, | ||
| "snippet": "<img src=\"blob:http://localhost:10200/822c70a0-b912-41c7-9a21-56c3d309e75b\">", | ||
| "snippet": "<img src=\"blob:http://localhost:10200/5a659d35-80ca-42e2-8f98-19daed3b98aa\">", |
There was a problem hiding this comment.
you got some churn here to manually remove
There was a problem hiding this comment.
you got some churn here to manually remove
it's only the four urls, so maybe just an optional nit?
| } | ||
|
|
||
| // Skip if we couldn't collect natural image size information. | ||
| if (!imageElement.naturalDimensions) continue; |
There was a problem hiding this comment.
ha, adapting existing code is kind of awkward, but this is really nice to have an explicit guard if there wasn't natural image size info and not have to worry about falsy values, etc
brendankenny
left a comment
brendankenny
left a comment
There was a problem hiding this comment.
thanks for getting this out so fast, @patrickhulce!
| return; | ||
| } | ||
| if (!size) return; | ||
| element.naturalDimensions = {width: size.naturalWidth, height: size.naturalHeight}; |
| "displayedWidth": 360, | ||
| "displayedHeight": 240, |
There was a problem hiding this comment.
things have changed a lot on dbw_tester since we last ran this, apparently :)
| "height": 16 | ||
| }, | ||
| "snippet": "<img src=\"blob:http://localhost:10200/822c70a0-b912-41c7-9a21-56c3d309e75b\">", | ||
| "snippet": "<img src=\"blob:http://localhost:10200/5a659d35-80ca-42e2-8f98-19daed3b98aa\">", |
There was a problem hiding this comment.
you got some churn here to manually remove
it's only the four urls, so maybe just an optional nit?
Co-authored-by: Connor Clark <[email protected]>
Co-authored-by: Connor Clark <[email protected]>
patrickhulce
force-pushed
the
image_v8_rework
branch
from
360683c to
0d37c29
Compare
4 participants
Summary
Reworks the ImageElements artifact for clarity and improved growth opportunities.
Primary changes:
cssWidth/cssHeighthave been removed.attributeWidth/attributeHeightcan now benullwhen the property is not set or if it's a CSS image (like in JSel.getAttribute.naturalWidth/naturalHeighthave been moved tonaturalDimensionsproperty that mirrors the intention behind_privateCssSizingand itsundefinedbehavior._privateCssSizing->cssEffectiveRules,sizingto me was ambiguous about the "effective source rule" portion and "rules" leaves the door open for future effective rules not related to sizingcssComputed*properties to acomputedStyleproperty.Related Issues/PRs
closes #12077
ref #11866