FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: implement css-variables and css-calc by m-abs · Pull Request #7553 · NativeScript/NativeScript · GitHub

/ NativeScript Public

feat: implement css-variables and css-calc - #7553

Merged
SvetoslavTsenov merged 50 commits into
NativeScript:masterfrom
m-abs:feat/css-variables
Aug 19, 2019
ConversationCommits (50)ChecksFiles changed
Merged

feat: implement css-variables and css-calc#7553
SvetoslavTsenov merged 50 commits into
NativeScript:masterfrom
m-abs:feat/css-variables

Conversation

m-abs commented Jul 19, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Checklist

What is the current behavior?

No support for css-variable or for css-calc

What is the new behavior?

Adds basic support for css-variable and css-calc

.button {
   --custom-color: black; /** text-color: black **/
   color: var(--custom-color);
}

.button.got-the-blues {
   --custom-color: blue; /** text-color: blue **/
}

.button.red {
   --custom-color: red;  /** text-color: red **/
}

Adds support for calc(...) in CSS, via reduce-css-calc.

StackLayout {
    --my-factor: 1;  /** width: 100% **/
   width: calc(100% * var(--my-factor));
}

StackLayout.slim {
    --my-factor: 0.1;  /** width: 10% **/
}

StackLayout.wide {
    --my-factor: 1.25; /** width: 125% **/
}

What is still missing:

  • Updating the variables via view.style is broken, if already defined in CSS.
  • For CSS-variables to be really useful, support for calc(var(--my-variable) * 2) is needed. I intend to add this to the PR.
  • clean up

Implements #4864

cla-bot Bot added the cla: yes label Jul 19, 2019
m-abs changed the title feat: implement basic support for css-variables feat: implement css-variables and css-calc Jul 19, 2019

exejutable commented Jul 24, 2019

Copy link
Copy Markdown

this is gonna be merged?

m-abs commented Jul 24, 2019

Copy link
Copy Markdown
Contributor Author

@exejutable
I hope so, but I need to do some clean up and it needs to be properly reviewed and tested.

themounthead commented Jul 30, 2019

Copy link
Copy Markdown

Can this merged please !!! I'm migrating my web app to a shared codebase and this is last missing piece to handle universal Theming.

bundyo commented Jul 30, 2019

Copy link
Copy Markdown
Contributor

I've tested this PR a bit and it seems to be working rather nice.

m-abs commented Aug 6, 2019

Copy link
Copy Markdown
Contributor Author

@manoldonev
Is it okay to depend on reduce-css-calc or should I try to write my own parser?

reduce-css-calc doesn't take up much space and work as advertised, but it is a new extra dependency plus its dependencies.

m-abs commented Aug 6, 2019

Copy link
Copy Markdown
Contributor Author

@manoldonev
I tried out my own implementation on https://github.com/m-abs/NativeScript/tree/feat/css-variables-own-calc.

Seems to work fine, but it needs a little clean up.

manoldonev added docs needed Additional documentation on this issue/PR is needed ♥ community PR labels Aug 15, 2019
manoldonev reviewed Aug 15, 2019
View reviewed changes
Comment thread tns-core-modules/ui/styling/style-scope.ts Outdated
manoldonev reviewed Aug 15, 2019
View reviewed changes
Comment thread tns-core-modules/ui/styling/style-scope.ts Outdated
manoldonev reviewed Aug 15, 2019
View reviewed changes
Comment thread tns-core-modules/ui/styling/style-scope.ts Outdated
manoldonev reviewed Aug 15, 2019
View reviewed changes
Comment thread tns-core-modules/ui/styling/style-scope.ts Outdated

manoldonev commented Aug 19, 2019

Copy link
Copy Markdown
Contributor

test

m-abs commented Aug 19, 2019

Copy link
Copy Markdown
Contributor Author

Hi @manoldonev,
Thank you for approving the PR.

Unfortunately I have discovered a limitation with the css-calc support.

Expressions like calc(100% - 30px + 20px) are reduced to calc(100% - 10px) because we don't know what the relative size of 100% is at the time.

Can we leave it as a known limitation and print a warning if/when it happens?

SvetoslavTsenov merged commit 673c808 into NativeScript:master Aug 19, 2019

SvetoslavTsenov commented Aug 19, 2019 •
edited by manoldonev
Loading

Copy link
Copy Markdown
Contributor

Hey @m-abs, this feat will be available in the next version of tns-core-modules@next and
in the next official version of tns-core-modules@6.1.0

m-abs deleted the feat/css-variables branch August 19, 2019 22:00

manoldonev commented Aug 20, 2019

Copy link
Copy Markdown
Contributor

@m-abs can you log this limitation #7553 (comment) as a separate issue and we will mark it with the "known issues" label for the time being?

manoldonev commented Aug 20, 2019

Copy link
Copy Markdown
Contributor

@m-abs Thank you for the awesome contribution and for taking time to address all comments!

m-abs mentioned this pull request Aug 20, 2019
Open
bundyo mentioned this pull request Aug 26, 2019
Open
45 tasks
CatchABus mentioned this pull request Mar 31, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
manoldonev manoldonev approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes ♥ community PR docs needed Additional documentation on this issue/PR is needed

Projects

NativeScript Docs Project
Status: 🆕 New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL