-
Notifications
You must be signed in to change notification settings - Fork 557
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
Add option to selectively include style properties when cloning element #436
base: master
Are you sure you want to change the base?
Conversation
* the full list of computedStyle properties can be cached * users can now manually specify which style properties are included
💖 Thanks for opening this pull request! 💖 Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #436 +/- ##
=======================================
Coverage 62.93% 62.93%
=======================================
Files 10 10
Lines 580 580
Branches 143 143
=======================================
Hits 365 365
Misses 151 151
Partials 64 64
☔ View full report in Codecov by Sentry. |
f612f36
to
f9efd26
Compare
* the full list of computedStyle properties can be cached * users can now manually specify which style properties are included This commit is applied from bubkoo/html-to-image#436
* the full list of computedStyle properties can be cached * users can now manually specify which style properties are included This commit is applied from bubkoo/html-to-image#436
* the full list of computedStyle properties can be cached This commit is applied from bubkoo/html-to-image#436
* the full list of computedStyle properties can be cached This commit is applied from bubkoo/html-to-image#436
@@ -118,7 +127,7 @@ function cloneCSSStyle<T extends HTMLElement>(nativeNode: T, clonedNode: T) { | |||
targetStyle.cssText = sourceStyle.cssText | |||
targetStyle.transformOrigin = sourceStyle.transformOrigin | |||
} else { | |||
toArray<string>(sourceStyle).forEach((name) => { | |||
getStyleProperties(options).forEach((name) => { |
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.
Instead of allocating a new array with all the property names for each node in order call .forEach
on it, it would be even faster to just use a good-old for
loop to iterate over all the properties. Since CSSStyleDeclaration
is "array-like", it works well:
const propertyNames = options.includeStyleProperties ?? sourceStyle;
for (let i = 0; i < propertyNames; ++i) {
const name = propertyNames[i];
let value = sourceStyle.getPropertyValue(name);
// ...
}
Description
Added a new option
includeStyleProperties
which allows users to selectively copy computed style.Motivation and Context
Currently we need to copy and serialize all the style properties in computedStyle for every node. In performance sensitive scenarios we would like to be able to minimize the time consumed when cloning node styles, for example:
pointer-events
,cursor
, etc.)Apart from performance use cases, as CSS custom properties are not iterable from
CSSStyleDeclaration
, they are not included in the snapshot image with current implementation (see #433). This new option may also help in this case as it allows users to manually specify custom properties that are used.Types of changes
Self Check before Merge