feat(styling): Added 2 functions to control applicationAdditionalSelectors - #6124
vakrilov merged 14 commits into
Conversation
Sorry, something went wrong.
|
test |
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
|
Hi @NathanaelA Failing unit test: Test: VIEW-LAYOUT-EVENT.test_event_LayoutChanged_IsNotRaised_TransformChanged FAILED: Actual: <0>(number). Expected: <1>(number) It might be some flakiness in unittest, passed after re-run. |
Sorry, something went wrong.
This comment was marked as abuse.
|
@NathanaelA could you rebase latest master onto this PR? |
Sorry, something went wrong.
There was a problem hiding this comment.
The 2 new methods seem useful for scenarios like the themes-switching, so 👍 the proposing them.
Can you add some unit-tests verifying that the methods work as expected? It is fairly isolated functionality so testing should be straight-forward.
You can extend the existing tests in style-tests.ts.
Sorry, something went wrong.
| changed = true; | ||
| } | ||
| } | ||
| return changed; |
There was a problem hiding this comment.
Probably you should call mergeCssSelectors(); here again if there are changes.
It will regenerate the applicationCssSelectors and will increment applicationCssSelectorVersion.
Sorry, something went wrong.
This comment was marked as abuse.
There was a problem hiding this comment.
Awesome!
For to operations (remove and after that add) the perf penalty won't be that big. If there are cases that you want to have multiple operations with tagged selectors - we can probably create an API that can batch these and call merge only once at the end.
Sorry, something went wrong.
This comment was marked as abuse.
|
test |
Sorry, something went wrong.
| const css1 = "Label { color: #FF0000; }"; | ||
| const css2 = "Label { color: #00FF00; }"; | ||
| const css1 = "#test_checkAddRemoveCSS_label { color: #FF0000; }"; | ||
| const css2 = "#test_checkAddRemoveCSS_label { color: #00FF00; }"; |
This comment was marked as abuse.
There was a problem hiding this comment.
buildUIAndRunTest will clear everything on the page (page-level CSS too), but will not clear the application-wide CSS selectors.
I noticed that the registered tag selectors remain which was affection other tests especially when running the tests with livesync. Thats why I made them specific id-selectors ;)
Sorry, something went wrong.
|
Sorry if this is a dumb question... but do the two added functions need to be added to style-scope.d.ts as well? |
Sorry, something went wrong.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sorry, something went wrong.
PR Checklist
What is the current behavior?
Somewhere between version 2.x of NativeScript and 4.x of NativeScript, the Styling system was changed. The application wide CSS selectors are no longer accessible from anything external to the style-scope file. (css selectors are now a local variable)
What is the new behavior?
I made a generic interface to be able to modify the applicationAdditionalSelectors basically I exposed two new functions off the StyleScope library;
This is a much better solution to solve #5912 (just exposing the applicationAdditionalSelectors) -- this will allow anyone to easily add more CSS text "globally" to the app that can be optionally removed at some point in the future -- because the rules are tagged; you can add any number of different rule sets, and remove any set of them on demand at a later time.
Fixes/Implements/Closes #5912
Please note to test the new functionality; you can use the new version of NativeScript-Themes and the demo it has. (You'll have to run from master).
This does not create any breaking changes; and unless the functions are actually utilized they do not have any impact on the existing code base. If they are utilized then they will impact just the long term global selectors.