-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat! Dropping Sass support from builtin video block #35506
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
Conversation
bd41dcd to
a509763
Compare
e4e5992 to
a5919c0
Compare
84e9274 to
19a8c75
Compare
|
Sandbox deployment successful 🚀 |
19a8c75 to
7dc8eae
Compare
|
Sandbox deployment failed 💥 |
|
Sandbox deployment failed 💥 |
d280339 to
7558ad4
Compare
|
Sandbox deployment successful 🚀 |
|
I am going to test this as its critical. |
|
I have tested Video Xblock, Things are working fine. |
|
@fayyazahmed66 Thanks for your valuable testing contribution, appreciated 🌟 |
|
I am going to review this PR. |
ttqureshi
left a comment
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 have reviewd the PR. It's good to go 🚀
salman2013
left a comment
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.
LGTM
7558ad4 to
a344a27
Compare
|
Sandbox deployment successful 🚀 |
kdmccormick
left a comment
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.
Just some comment corrections.
If I'm merging this PR later in the day anyway, I can apply these before merging.
8f58637 to
7764e47
Compare
|
Sandbox deployment successful 🚀 |
|
Sandbox deployment successful 🚀 |
kdmccormick
left a comment
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.
Great work! 🚀
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Dropping Sass support from builtin video block
Tickets: #35570
Tasks:
Convert Sass variable into css variables
Compile the css files of the block following below steps.
xmodule/static/css-builtin-blocks.Replace
add_sass_to_fragmenttoadd_css_to_fragment in blocksin.pyfileRemove all .scss files linked to the block under xmodule/assets.
Testing:
npm run buildto run webpack and compile sass files../manage.py lms collectstaticin lms shell to re-collect static files../manage.py cms collectstaticin cms shell to re-collect static files.