-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[IMPL] - added support for sass and scss stylesheet languages #4292
Open
KronosDev-Pro
wants to merge
28
commits into
reflex-dev:main
Choose a base branch
from
KronosDev-Pro:add-sass-scss-stylesheet-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+155
−12
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
0681cc2
[IMPL] - added support for sass and scss stylesheet languages
KronosDev-Pro 707f345
fix f-string bug
KronosDev-Pro e645d0f
make "libsass" an optional dependency
KronosDev-Pro 7e7687d
remove libsass of deps list
KronosDev-Pro d6bd371
revert peotry relock
KronosDev-Pro 0fe38d2
fix test caused by optional "libsass" deps
KronosDev-Pro df9818c
improving `_compile_root_stylesheet` function and add folder styleshe…
KronosDev-Pro 370b138
fix the copy files in assets to public folder
KronosDev-Pro 9bdea04
Merge branch 'main' into add-sass-scss-stylesheet-support
KronosDev-Pro 4df9d5d
remove useless f-string
KronosDev-Pro c523872
little general improvement
KronosDev-Pro 13b21a1
fix f-string
KronosDev-Pro 0223081
remove useless path search
KronosDev-Pro 4fae418
remove unused var & import
KronosDev-Pro 613e130
[IMPL] - added support for sass and scss stylesheet languages
KronosDev-Pro 81dab90
fix f-string bug
KronosDev-Pro e815903
make "libsass" an optional dependency
KronosDev-Pro 6e32fcb
remove libsass of deps list
KronosDev-Pro 2ca363c
fix test caused by optional "libsass" deps
KronosDev-Pro 6898a26
improving `_compile_root_stylesheet` function and add folder styleshe…
KronosDev-Pro 59a0cbf
fix the copy files in assets to public folder
KronosDev-Pro 6a2a5ff
remove useless f-string
KronosDev-Pro f04431a
little general improvement
KronosDev-Pro 3b403cc
fix f-string
KronosDev-Pro b8e2ddd
remove useless path search
KronosDev-Pro 8b46852
remove unused var & import
KronosDev-Pro ff18003
Merge branch 'add-sass-scss-stylesheet-support' of https://github.com…
KronosDev-Pro 0dde556
Merge branch 'main' into add-sass-scss-stylesheet-support
KronosDev-Pro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,7 +106,7 @@ def test_compile_imports(import_dict: ParsedImportDict, test_dicts: List[dict]): | |
assert sorted(import_dict["rest"]) == test_dict["rest"] # type: ignore | ||
|
||
|
||
def test_compile_stylesheets(tmp_path, mocker): | ||
def test_compile_stylesheets(tmp_path: Path, mocker): | ||
"""Test that stylesheets compile correctly. | ||
|
||
Args: | ||
|
@@ -119,7 +119,9 @@ def test_compile_stylesheets(tmp_path, mocker): | |
assets_dir = project / "assets" | ||
assets_dir.mkdir() | ||
|
||
(assets_dir / "styles.css").touch() | ||
(assets_dir / "styles.css").write_text( | ||
"button.rt-Button {\n\tborder-radius:unset !important;\n}" | ||
) | ||
mocker.patch("reflex.compiler.compiler.Path.cwd", return_value=project) | ||
|
||
stylesheets = [ | ||
|
@@ -134,10 +136,82 @@ def test_compile_stylesheets(tmp_path, mocker): | |
"@import url('./tailwind.css'); \n" | ||
"@import url('https://fonts.googleapis.com/css?family=Sofia&effect=neon|outline|emboss|shadow-multiple'); \n" | ||
"@import url('https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css'); \n" | ||
"@import url('../public/styles.css'); \n" | ||
"@import url('./styles.css'); \n" | ||
"@import url('https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap-theme.min.css'); \n", | ||
) | ||
|
||
assert (project / ".web" / "styles" / "styles.css").read_text() == ( | ||
assets_dir / "styles.css" | ||
).read_text() | ||
|
||
|
||
def test_compile_stylesheets_scss_sass(tmp_path: Path, mocker): | ||
try: | ||
import sass # noqa: F401 | ||
except ImportError: | ||
pytest.skip( | ||
'The `libsass` package is required to compile sass/scss stylesheet files. Run `pip install "libsass>=0.23.0"`.' | ||
) | ||
|
||
project = tmp_path / "test_project" | ||
project.mkdir() | ||
|
||
assets_dir = project / "assets" | ||
assets_dir.mkdir() | ||
|
||
assets_preprocess_dir = assets_dir / "preprocess" | ||
assets_preprocess_dir.mkdir() | ||
|
||
(assets_dir / "styles.css").write_text( | ||
"button.rt-Button {\n\tborder-radius:unset !important;\n}" | ||
) | ||
(assets_preprocess_dir / "styles_a.sass").write_text( | ||
"button.rt-Button\n\tborder-radius:unset !important" | ||
) | ||
(assets_preprocess_dir / "styles_b.scss").write_text( | ||
"button.rt-Button {\n\tborder-radius:unset !important;\n}" | ||
) | ||
mocker.patch("reflex.compiler.compiler.Path.cwd", return_value=project) | ||
|
||
stylesheets = [ | ||
"/styles.css", | ||
"/preprocess/styles_a.sass", | ||
"/preprocess/styles_b.scss", | ||
] | ||
|
||
assert compiler.compile_root_stylesheet(stylesheets) == ( | ||
str(Path(".web") / "styles" / "styles.css"), | ||
"@import url('./tailwind.css'); \n" | ||
"@import url('./styles.css'); \n" | ||
"@import url('./preprocess/styles_a.css'); \n" | ||
"@import url('./preprocess/styles_b.css'); \n", | ||
) | ||
|
||
stylesheets = [ | ||
"/styles.css", | ||
"/preprocess", # this is a folder containing "styles_a.sass" and "styles_b.scss" | ||
] | ||
|
||
assert compiler.compile_root_stylesheet(stylesheets) == ( | ||
str(Path(".web") / "styles" / "styles.css"), | ||
"@import url('./tailwind.css'); \n" | ||
"@import url('./styles.css'); \n" | ||
"@import url('./preprocess/styles_b.css'); \n" | ||
"@import url('./preprocess/styles_a.css'); \n", | ||
) | ||
|
||
assert (project / ".web" / "styles" / "styles.css").read_text() == ( | ||
assets_dir / "styles.css" | ||
).read_text() | ||
|
||
expected_result = "button.rt-Button{border-radius:unset !important}\n" | ||
assert ( | ||
project / ".web" / "styles" / "preprocess" / "styles_a.css" | ||
).read_text() == expected_result | ||
assert ( | ||
project / ".web" / "styles" / "preprocess" / "styles_b.css" | ||
).read_text() == expected_result | ||
|
||
|
||
def test_compile_stylesheets_exclude_tailwind(tmp_path, mocker): | ||
"""Test that Tailwind is excluded if tailwind config is explicitly set to None. | ||
|
@@ -165,7 +239,7 @@ def test_compile_stylesheets_exclude_tailwind(tmp_path, mocker): | |
|
||
assert compiler.compile_root_stylesheet(stylesheets) == ( | ||
str(Path(".web") / "styles" / "styles.css"), | ||
"@import url('../public/styles.css'); \n", | ||
"@import url('./styles.css'); \n", | ||
) | ||
|
||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure this won't break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where this could cause a problem, since all the supported stylesheet files in the
app/assets
directories and subdirectories are copied or compiled and saved in the.web/styles
directory and no longer in the.web/public
directories.and also, I checked the second batch of the
tests/integration
group and thetests/integration/test_tailwind.py
(which is the only one to have tests impacted by the changes) passed the tests without any errors, which is why I don't understand the error in the first batch of thetests/integration
group.