feat: Expose xmodule xblocks Sass variables as vanilla CSS variables

[farhan]

Ticket: #35173

Following script is used to extract out the required sass variables to expose:**

# Search for all Sass var references, sort them, and save them to a file.
rg '\$[a-zA-Z0-9-_]*'   xmodule/assets/**/*scss --no-line-number --no-filename --only-matching \
    | sort | uniq > variable_references.txt

# Search for local declarations of Sass vars (like '$selected_color: ...'), sort them,
# and save them to a file.
rg '\$[a-zA-Z0-9-_]*: ' xmodule/assets/**/*scss --no-line-number --no-filename --only-matching \
    | sed 's/..$//' | sort | uniq > local_variable_declarations.txt

# Compare the set of var references with the set of local declarations.
# Variables which are referenced but *don't* have local declarations must be themeable Sass variable.
# Save them to a file.
diff --unified local_variable_declarations.txt variable_references.txt \
    | rg '^\+' | sed 's/^.//'  > themable_variables.txt

How to test:

Test in xmodule xblocks:

1. Checkout the PR branch
2. Run npm run compile-sass or npm run compile-sass-dev in lms shell. Restart lms if changes don't reflect.
3. Open any XBlock in the lms or cms. Open dev tools window and find the css file in the scripts window. Select the xblock first with the Inspect selector arrow.
4. We can also hit following command in console to verify the variable

getComputedStyle(document.documentElement).getPropertyValue('--baseline')

Test in extracted xblocks:

1. Checkout the extracted word-cloud xblocks PR. Follow instructions on the PR.
2. Convert the word-cloud xblock hard code variable into css global variable in file as shared below

.input-cloud {
    margin: var(--baseline)/4
    /*margin: 5px;*/
}

3. Install the xblock into the lms and cms. Restart both services.
4. Now Open the installed xblock in the lms or cms and find out the value changed in the second step.
5. We can also hit following command in console to verify the variable

getComputedStyle(document.documentElement).getPropertyValue('--baseline')

[kdmccormick]

Looking great so far @farhan . Does this capture every Sass variable that used within xmodule/assets?

Also, I just edited the issue to include a Step 4:

4. For each builtin block....
   • Modify the block's Sass code to use the CSS variables instead of the Sass variables.
   • Tip: Start with one block and merge the PR. That way, if it breaks on edX or in Tutor, we know sooner than later.

Could you choose a builtin block, and apply Step 4 for that block in this PR?

There will also be "Step 5: Convert each blocks' Sass into CSS", but I need to do some discovery before I can provide details for that. Let's worry about Step 5 in a future PR.

[farhan]

@kdmccormick

Does this capture every Sass variable that used within xmodule/assets?

No, it doesn't cover all the sass variables, we are working on it. Variables like Annotators xblock's need to be discussed with @irtazaakram

There will also be "Step 5: Convert each blocks' Sass into CSS", but I need to do some discovery before I can provide details for that. Let's worry about Step 5 in a future PR.

I have converted the word-cloud xblock sass variable into css global variable as below looking into the file and it's working fine

.input-cloud {
    margin: var(--baseline)/4
    /*margin: 5px;*/
}

[farhan]

@kdmccormick
We have added all the sass variables now. update file

It includes all the sass variables directly import from the imports excluding the one locally defined in the files.
cc: @irtazaakram

[kdmccormick]

Great. Let me know when you are ready for review.

[kdmccormick]

I believe these Sass variables are missing from _builtin-block-variables.scss:

$all-text-inputs
$base-font-size
$base-line-height
$black-t2
$bp-screen-lg
$btn-brand-focus-background
$correct
$danger
$font-bold
$general-color-accent
$gray-300
$gray-d1
$highlight
$incorrect
$medium-font-size
$partially-correct
$small-font-size
$submitted
$success
$tmg-s2
$uxpl-gray-background
$uxpl-gray-base
$uxpl-gray-dark
$warning
$warning-color
$warning-color-accent
$yellow

Here is the bash script I used to determine that (it requires ripgrep, also known as rg):

# Search for all Sass var references, sort them, and save them to a file.
rg '\$[a-zA-Z0-9-_]*'   xmodule/assets/**/*scss --no-line-number --no-filename --only-matching \
    | sort | uniq > variable_references.txt

# Search for local declarations of Sass vars (like '$selected_color: ...'), sort them,
# and save them to a file.
rg '\$[a-zA-Z0-9-_]*: ' xmodule/assets/**/*scss --no-line-number --no-filename --only-matching \
    | sed 's/..$//' | sort | uniq > local_variable_declarations.txt

# Compare the set of var references with the set of local declarations.
# Variables which are referenced but *don't* have local declarations must be themeable Sass variable.
# Save them to a file.
diff --unified local_variable_declarations.txt variable_references.txt \
    | rg '^\+' | sed 's/^.//'  > themable_variables.txt

# Search the new _builtin-block-variables.scss file to get a list of CSS->Sass mapped variables.
# Sort and save them to a file.
rg '\$[A-Za-z0-9-_]*' common/static/sass/_builtin-block-variables.scss --no-line-number --no-filename --only-matching \
    | sort | uniq > css_mapped_variables.txt

# Finally, compare the CSS->Sass-mapped variables with the themed ones we saved earlier.
# This is the list of variables that we are missing CSS->Sass mappings for.
diff --unified css_mapped_variables.txt themable_variables.txt \
    | rg '^\+' | sed 's/^.//'

I think it would be best to get all these variables into _builtin-block-variables.scss now so that we don't have bugs later. Let me know if you are curious how any part of that bash script works-- I find bash really helpful when I have to do these sort of tasks and I'd be happy to share the knowledge.

[kdmccormick]

(see my comment above)

[farhan]

@kdmccormick Thanks for sharing ripgrep scripts, it's really helpful and I learned about a very handy library to deal with these kinds of stuff.

FYI: I have removed $highlight as its a local variable used in loop so couldn't come under $*: regex

PR is available for review again

[kdmccormick]

Looks great, thanks @farhan !

Can you explain how you manually tested this to ensure that the CSS variables are available in legacy views?

[farhan]

Can you explain how you manually tested this to ensure that the CSS variables are available in legacy views?

@kdmccormick I have added my testing steps in the description of the PR. Hope this is what you are asking for.
I didn't get what is meant by legacy views

[kdmccormick]

Forgive me @farhan , when I said "legacy views" what I should have said is "server-rendered views" (as opposed to MFE pages). XBlocks fall into that category, so your testing steps look perfect 👍🏻 🚀

[kdmccormick]

When you squash & merge, please include a link to the ticket in the commit message.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[e0d]

Late the party, but can't this | sort | uniq just be | sort -u?

[kdmccormick]

TIL 😄