Skip to content
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

Implement style computing #1399

Merged
merged 9 commits into from Mar 4, 2021
Merged

Implement style computing #1399

merged 9 commits into from Mar 4, 2021

Conversation

TrySound
Copy link
Member

@TrySound TrySound commented Mar 2, 2021

Ref #777

Currently a lot of optimisations are attributes specific and may be
broken because of inline or shared styles.

In this diff I'm trying to solve the problem with getComputedStyle
analog.

computeStyle collects attributes, shared css rules, inline styles
and inherited styles and checks whether they can be statically optimised
or left as deoptimisation.

As an example I integrated computeStyle into removeHiddenElems plugin
which is fixed bugs with styles in style rules and inline styles.
Note: review with hidden whitespaces

Ref #777

Currently a lot of optimisations are attributes specific and may be
broken because of inline or shared styles.

In this diff I'm trying to solve the problem with getComputedStyle
analog.

`computeStyle` collects attributes, shared css rules, inline styles
and inherited styles and checks whether they can be statically optimised
or left as deoptimisation.
@TrySound TrySound marked this pull request as ready for review March 3, 2021 15:07
@TrySound
Copy link
Member Author

TrySound commented Mar 3, 2021

cc @strarsis @XhmikosR @caub @sk-

@strarsis
Copy link
Contributor

strarsis commented Mar 3, 2021

The inlineStyles plugin already does this, it collects the styles and then stably sorts them which surprisingly is sufficient. Just selector-based filtering would be needed to compute the styles of a particular element, which is also easily possible now. It should be possible to extract this part into a dedicated function (css tools module). As a performance optimization it would be also possible to provide a prepared presorted structure as the plugin does so this doesn't has to be performed each time. Alternatively transparent caching could be used (as it was added for element styles support).

@TrySound
Copy link
Member Author

TrySound commented Mar 3, 2021

Proper caching will require some architecture changes. First we should fix bugs.

And yeah, I saw inlineStyles though the point here is resolving style from all sources including inheritance and presentation attributes.

All plugins should work correctly in isolation. So caching will be implemented on core level not in plugin.

@caub
Copy link
Contributor

caub commented Mar 3, 2021

looks really good 👍🏽 from the tests, never thought getComputedStyle could be done in this quite short way

@TrySound
Copy link
Member Author

TrySound commented Mar 3, 2021

Me neither 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants