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

Sass bugs fixing #167

Merged
merged 8 commits into from
Jul 2, 2022
Merged

Sass bugs fixing #167

merged 8 commits into from
Jul 2, 2022

Conversation

ntt261298
Copy link
Collaborator

@ntt261298 ntt261298 commented Jun 28, 2022

Related issues

Fixes

  • Cannot process scss < 1.45.0
  • Create react app's example can not load SASS_PATH
    • Reason: Path contains \ before _ (\ is automatically added before _ when pressing save)
    • Solution: Temporarily disable auto-format on saving to be able to commit the change
    • Concern: Not sure which rule triggers that (adding \), we might change .env file in the future and someone will commit the change (Added comment to notice this)

Chores

  • Make example @emotion/react noticeable: Currently when hovering over the button, the color is still remaining white, so it's not noticeable => Change text color to blue

@netlify
Copy link

netlify bot commented Jun 28, 2022

Deploy Preview for jest-preview-library canceled.

Name Link
🔨 Latest commit 925e10d
🔍 Latest deploy log https://app.netlify.com/sites/jest-preview-library/deploys/62c06b31347d8000088f569f

@nvh95
Copy link
Owner

nvh95 commented Jul 1, 2022

@ntt261298 I think we can solve 2 issues by 2 different PRs, so we can ship it faster. What do you think?
If so, please update the PR description. Thanks.

@ntt261298 ntt261298 marked this pull request as ready for review July 1, 2022 16:29
@ntt261298
Copy link
Collaborator Author

@ntt261298 I think we can solve 2 issues by 2 different PRs, so we can ship it faster. What do you think? If so, please update the PR description. Thanks.

I just update the description to include only 1 issue. Please help to take a look at the update. Thanks a lot.

],
})
.css.toString();
}
Copy link
Owner

@nvh95 nvh95 Jul 1, 2022

Choose a reason for hiding this comment

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

@ntt261298 Can we just throw an error in else block?

@nvh95
Copy link
Owner

nvh95 commented Jul 1, 2022

@ntt261298 I think we should have 2 tests to make sure sass work with version < 1.45.0 and >= 1.45.0. Let's brainstorm together.

Copy link
Owner

@nvh95 nvh95 left a comment

Choose a reason for hiding this comment

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

LGTM!

@nvh95 nvh95 merged commit 6d12470 into main Jul 2, 2022
@nvh95 nvh95 deleted the sass-bugs-fixing branch July 2, 2022 16:02
@nvh95 nvh95 added this to the 0.2.6 milestone Jul 2, 2022
@nvh95 nvh95 linked an issue Jul 2, 2022 that may be closed by this pull request
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.

Cannot process scss < 1.45.0
2 participants