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

DEMO: Export SCSS to JS #76

Closed
wants to merge 1 commit into from
Closed

Conversation

ecgan
Copy link
Member

@ecgan ecgan commented Nov 26, 2020

Changes proposed in this Pull Request:

This is just a demo PR for discussion, not meant to be merged.

Some online source:

Apparently we can only export SCSS variables (the ones with $ prefix). There isn't a way to export mixins.

This video shows manually creating JavaScript mixin function to replace SCSS mixin: https://egghead.io/lessons/scss-use-javascript-functions-as-mixin-directives-for-css-in-js-style-declarations

Screenshots:

image

Detailed test instructions:

  1. Go to http://localhost:8888/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fstart
  2. Notice the small output at the bottom of the screen as per screenshot.
  3. In the console, you can see the exported object.

@ecgan ecgan requested review from jconroy, tomalec and a team November 26, 2020 16:31
@import "node_modules/@wordpress/base-styles/default-custom-properties";

:export {
black: $black;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we still need to manually map everything like this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, which means we probably can't go CSS-in-JS / emotion library 100%, instead we might have to use emotion when possible and SCSS when needed until the wp-g2 CSS-in-JS matures.

Copy link
Member

@jconroy jconroy Nov 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally rather us be able to use a single approach. Mixing approaches for anything almost certainly from my experience will be a headache/nightmare. In which case I'd suggest we stick with scss for now and put a pin in the CSS in JS convo (again, just for now),

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, if that's the case, we gonna set our expectations that there will be ongoing arguments on CSS class naming conventions between me and @tomalec 🤣 OK, stick with current SCSS way for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ongoing arguments on CSS class naming conventions

An exchange of views among peers 😄

If we stick to something based around hopefully we won't get into too much trouble and/or have to spend too much time thinking of the perfect name

If we hit uniqueness issues we can revisit.

@ecgan ecgan self-assigned this Nov 27, 2020
@ecgan ecgan closed this Nov 27, 2020
@eason9487 eason9487 deleted the feature/export-scss-to-js branch February 21, 2023 07:28
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.

2 participants