Skip to content

refactor(css): attribute selectors match web counterparts#7848

Merged
vtrifonov merged 16 commits intomasterfrom
bundev/css-selector-optimization
Mar 26, 2020
Merged

refactor(css): attribute selectors match web counterparts#7848
vtrifonov merged 16 commits intomasterfrom
bundev/css-selector-optimization

Conversation

@bundyo
Copy link
Copy Markdown
Contributor

@bundyo bundyo commented Sep 20, 2019

PR Checklist

What is the current behavior?

Since the new Theme 2.x produces bigger CSS and uses Element selector styling, the matched elements on the page are more and performance suffers. There are demo projects in the linked issue.

What is the new behavior?

Exclude ProxyViewContainer from the CSS selector matching/init. Optimize some of the matchers and the selector query. Rework the attribute selectors to use exact matching, instead of regular expressions (as is done in browsers).

Fixes/Implements/Closes NativeScript/theme#179.

BREAKING CHANGES:

Classes set on ProxyViewContainer will probably not work anymore. We might consider moving them to the wrapped element - either automatically in core modules or by the user?

  with some JavaScript optimization and excluding ProxyViewContainer from the process
Change the specificity to be divisible to 10
@bundyo bundyo self-assigned this Sep 20, 2019
@cla-bot cla-bot bot added the cla: yes label Sep 20, 2019
@MartoYankov
Copy link
Copy Markdown
Contributor

@manoldonev Can you take a look at this? I couldn't find a scenario where there are significant performance gains (the projects mentioned had a different bug), but I think it's good even if only as a refactoring.

@edusperoni
Copy link
Copy Markdown
Contributor

I'm working in allowing some layout properties to cascade from ProxyViewContainer to the children, so this breaking change will potentially make it impossible to implement something like CSS Grid. Can this breaking change be clarified or maybe worked around?

@bundyo
Copy link
Copy Markdown
Contributor Author

bundyo commented Oct 23, 2019

The main issue turned out to be elsewhere, so this PR can be reduced to only get the attribute selectors match their web counterparts.

@manoldonev
Copy link
Copy Markdown
Contributor

@edusperoni Can you elaborate on your concern about the ProxyViewContainer change? Until now we have not actually considered setting css classes on ProxyViewContainer as a meaningful supported scenario. Are you actually trying to set such css classes or you are concerned that cascading will not work? Based on the current change I think cascading should continue to work?

@edusperoni
Copy link
Copy Markdown
Contributor

@manoldonev Please see the discussion in NativeScript/nativescript-angular#1422 (comment)

My current plan is, once we settle on the implementation details of forwarding properties to children, to do:

<GridLayout class="grid-container">
  <my-component class="grid-header"></my-component>
  <my-component class="grid-left"></my-component>
  <my-component class="grid-content"></my-component>
  <my-component class="grid-footer"></my-component>
</GridLayout>
.grid-container {
   grid-template-areas: 'header header' 'left content' 'footer footer';
}

.grid-header {
   grid-area: header;
}
.grid-left {
   grid-area: left;
}
.grid-content {
   grid-area: content;
}
.grid-footer {
   grid-area: footer;
}

GridLayout would then take the children styles and apply row/col to them as required. If we remove the ability for ProxyViewContainer to be matched and properly hold styles, we'll no longer be able to implement grid or add properties that will be forwarded to children (like row/col) in CSS

@manoldonev manoldonev changed the title fix(css): Improve CSS selector parsing/matching speed refactor(css): attribute selectors match web counterparts Nov 21, 2019
@vtrifonov vtrifonov merged commit c9cea47 into master Mar 26, 2020
@vtrifonov vtrifonov deleted the bundev/css-selector-optimization branch March 26, 2020 17:04
NathanWalker pushed a commit that referenced this pull request Aug 7, 2020
* Improve CSS selector parsing/matching by 30% - 40%
  with some JavaScript optimization and excluding ProxyViewContainer from the process
Change the specificity to be divisible to 10

* fix: selector match

* fix: lint errors

* refactor: restore processing of ProxyViewContainer

* chore: lower the number of expected cycles

* fix: some css selector fixes

Co-authored-by: Manol Donev <manol.donev@gmail.com>
Co-authored-by: Manol Donev <manoldonev@users.noreply.github.com>
Co-authored-by: Vasil Trifonov <v.trifonov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strange performance issue in 2.0 when rendering Angular components for the first time

6 participants