Add styles/variables for two-column block

[samuelallan72]

Description

Add styles and variables used by the two-column block. See opencraft/dev/exemplar-blocks!28 for how this is used.

Testing instructions

See opencraft/dev/exemplar-blocks!28

Supporting information

Private-ref: https://tasks.opencraft.com/browse/BB-10774

[xitij2000]

Putting it in tokens/core will add the prefix "pgn". Instead you should put it in apps/two-column/file.toml to not get a prefix. More here: https://github.com/openedx/paragon/tree/release-23.x/tokens#application-tokens

This is a very recent addition to paragon, just a week or two old!

[samuelallan72]

@xitij2000 ah cool, good to know thanks 👍

[samuelallan72]

@xitij2000 unfortunately it doesn't work - if I move core/two-column-block.toml to apps/two-column-block/main.toml, I get this error:

two.column.block.image.size.border.radius.lg.$value tries to reference wgu.size.border.radius.m, which is not defined.
two.column.block.image.size.border.radius.sm.$value tries to reference wgu.size.border.radius.xs, which is not defined.

[xitij2000]

So this seems to be a shortcoming of Paragon. I've contributed a fix here: openedx/paragon#4303

The issue is that paragon only looks at its own token set for tokens in the apps directory, so it doesn't see any custom tokens in our theme and considers it an error if we use them.

You can kind of solve this by using the css variable value, i.e.:

[two-column-block.image.size.border.radius]
"$value" = "var(--pgn-wgu-size-border-radius.m)"

Note: While the output variable will be the same, you should use "two-column-block...." rather than "two.column.block..." since "two-column-block" is an entity of its own.

[samuelallan72]

@xitij2000 ah nice, thanks!

Note: While the output variable will be the same, you should use "two-column-block...." rather than "two.column.block..." since "two-column-block" is an entity of its own.

Ah got it, that makes sense. I'll update that here. :)

[xitij2000]

This should not have rem, it should be unitless.

[samuelallan72]

Hmm unitless doesn't actually work, as it auto-adds a px postfix:

diff --git a/paragon/css/core/variables.css b/paragon/css/core/variables.css
index 2ba1ced..f551784 100644
--- a/paragon/css/core/variables.css
+++ b/paragon/css/core/variables.css
@@ -301,7 +301,7 @@
   --pgn-typography-badge-font-size: 75%;
   --pgn-typography-btn-font-size-base: 1rem;
   --pgn-typography-btn-font-weight: 700;
-  --pgn-typography-btn-line-height-base: 1.5rem;
+  --pgn-typography-btn-line-height-base: 1.5px;
   --pgn-typography-dropdown-item-text-decoration: none;
   --pgn-typography-image-figure-caption-font-size: 90%;
   --pgn-typography-menu-select-btn-link-text-decoration-line: underline;
diff --git a/tokens/core/typography.toml b/tokens/core/typography.toml
index c970ddc..527af90 100644
--- a/tokens/core/typography.toml
+++ b/tokens/core/typography.toml
@@ -32,4 +32,4 @@
 "$value" = "700"
 
 [typography.btn.line.height.base]
-"$value" = "1.5rem"
+"$value" = "1.5"

Also, either way the value set for typography.btn.line.height.base is ignored, because --pgn-typography-btn-line-height-base: var(--pgn-typography-input-btn-line-height-base); is output to variables.css further down in the file.

So I can't override the button line height. Bug in paragon? @xitij2000

[samuelallan72]

For now I dropped the button line-height. The default I think is close enough for now. There are higher priority things to tackle.

[xitij2000]

@samuelallan72 Ah! I completely missed this but it should be typography.btn.line-height.base and not typography.btn.line.height.base. i.e. it should be line-height with a dash in the middle and not line.height The issue is they both result in the same output variable. Once make this change, both the issues will be resolved.

It's duplicating the variable since the two inputs are different variables, but they output to the same final variable name.

As for the px issue. The correct path typography.btn.line-height.base has its type set to "number", whereas by default it's guessing the type to be "size" and automatically appending the px.

[samuelallan72]

@xitij2000 lol I see, thanks for picking that up!

[xitij2000]

I wish there was some way to warn about this. I've made a similar mistake and it was hard to catch, especially if you're starting with the variable name,

[xitij2000]

@samuelallan72 I think we can merge this after the conflicts are resolved.

[samuelallan72]

side thought: it takes > 5 minutes to run npm run build:scss in this repo, which feels extremely slow. It would be nice to optimise this at some point. cc @xitij2000

[samuelallan72]

@xitij2000 I've rebased, fixed the conflicts, and addressed your comments :) Would you mind leaving an approval if you're still happy with it?

[xitij2000]

@samuelallan72 Approved and merged!

For speeding up the build, we can use --source-tokens-only This is what the watch mode used during development to make the changes almost instant. I think that output that produces should be equivalent in our use-case, that should run in seconds since it'll only process the tokens we add.