BP-1293: Add theming support to new rich-text editor for basic redactor configuration #240

jovana-marceta · 2024-10-30T08:53:42Z

https://enigmatry.atlassian.net/browse/BP-1293

The Redactor editor will replace CKEditor. This new rich text editor includes predefined CSS that lacks support for our custom theming system, which covers colors, fonts, spacing, and other customizable options.

To address this, this update introduces a mixin that overrides default values for the following elements used in the Basics & Standard Redactor configurations.

Covered elements: Headings (h1 to h6), toolbar buttons, paragraphs, bullet and numbered lists, checkboxes (to-do items), tables, buttons, and links.

Additionally, our custom Redactor class and light mode by default have been added to the Redactor repository.

libs/entry-components/styles/modules/vendors/redactor/_buttons-generator.scss

libs/entry-components/styles/partials/theming.scss

libs/entry-components/styles/modules/vendors/redactor/_tables-generator.scss

libs/entry-components/styles/_generator.scss

libs/entry-components/styles/modules/vendors/redactor/_buttons-generator.scss

libs/entry-components/styles/modules/vendors/redactor/_checkboxes-generator.scss

libs/entry-components/styles/partials/vendors/overrides/redactor/_tables.scss

libs/entry-components/styles/partials/vendors/overrides/redactor/_general.scss

libs/entry-components/styles/partials/vendors/overrides/redactor/_buttons.scss

libs/entry-components/styles/modules/vendors/redactor/_buttons-generator.scss

libs/entry-components/styles/modules/vendors/redactor/_tables-generator.scss

libs/entry-components/styles/partials/vendors/overrides/redactor/_general.scss

…s for rich text

libs/entry-components/styles/partials/vendors/overrides/rich-text/_buttons.scss

libs/entry-components/styles/partials/vendors/overrides/rich-text/_forms.scss

jovana-marceta · 2024-11-14T11:55:01Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_typography-generator.scss

+
+	.enigmatry-redactor-content {
+		.rx-editor, .rx-content, .rx-content p {
+			@if $theming-fonts and map.has-key($theming-fonts, 'body') and map.get($theming-fonts, 'body', 'color') != null {


I attempted various approaches to avoid the null value and bypass the stylelint rule, but unfortunately, none of them were successful.

map.get($theming-fonts, 'body', 'color') != null fails to compile without previous checks if i.e. $theming-font is unset or there is no body defined in it?

typhoon41 · 2024-11-15T15:00:29Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_buttons-generator.scss

+	.enigmatry-redactor-content {
+		button, .rx-content button {
+
+			@if (theming-buttons, 'colors', 'primary') {


I'm not aware of this comparison. Can you give us as some example of docs from which you learned this?

I made a mistake and forgot to add the $ sign to the theming-buttons variable. Condition should look like this:
@if (map.get($theme, 'general', 'fonts', 'buttons'), 'colors', 'primary')

That's fine. Short and sweat, but where did you find it? Nothing about that in official docs AFAIK?

The ability to pass multiple keys to map.get is a feature supported in Dart Sass, as described in the official Sass documentation. This functionality allows for concise access to deeply nested values in maps without chaining multiple map.get calls manually. It simplifies the process of working with complex nested data structures like our $theme map

This feature is documented in the official Sass sass:map module

Additionally, here are GitHub issuse tracking the feature:
sass/sass#1739
sass/dart-sass#1884

This is not about map.get keys. We're using that on various places. The question was about if condition joined with such map syntax?
IDK how this would even compile: @if ($theming-buttons, 'colors', 'primary')

Is the intention to map.get primary color and then compare it with unset (null) value?

typhoon41 · 2024-11-15T15:01:19Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_buttons-generator.scss

+
+		.rx-toolbar-buttons .rx-button {
+			&.active, &:hover {
+				@if map.get($theme, 'general', 'inputs', 'background') {


This is duplicated extraction. We can just do it once and make null comparison if unset.

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_checkboxes-generator.scss

typhoon41 · 2024-11-15T15:11:42Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_tables-generator.scss

+				}
+			}
+
+			@if (map.get($theme, 'tables', 'header', 'font-size')) {


Duplicated retrieval.

typhoon41 · 2024-11-15T15:14:02Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_tables-generator.scss

+
+		table, table td, table th {
+			/* stylelint-disable-next-line scss/at-if-no-null */
+			@if $theming-fonts and map.has-key($theming-fonts, 'body') and map.get($theming-fonts, 'body', 'size') != null {


Will it fail compiling if we just check directly size from body (without has key and previous check)?
We would then get rid of duplicated retrieval.

Still legit.

typhoon41 · 2024-11-15T15:14:27Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_tables-generator.scss

+
+			#{$redactor-th-selector} {
+				/* stylelint-disable-next-line scss/at-if-no-null */
+				@if map.has-key($theme, 'tables') and map.has-key(map.get($theme, 'tables', 'header'), 'font-size')


The same applies for this one.

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_toolbar-generator.scss

typhoon41 · 2024-11-15T15:16:24Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_typography-generator.scss

+
+	.enigmatry-redactor-content {
+		.rx-editor, .rx-content, .rx-content p {
+			@if $theming-fonts and map.has-key($theming-fonts, 'body') and map.get($theming-fonts, 'body', 'color') != null {


map.get($theming-fonts, 'body', 'color') != null fails to compile without previous checks if i.e. $theming-font is unset or there is no body defined in it?

typhoon41 · 2024-11-15T15:30:52Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_typography-generator.scss

+			@else {
+				font-family: map.get($body-typography, 'font-family');
+			}
+


This is too much duplication and hard to follow. Maybe an option to change this is to replace with the following:

pseudocode -
Get the body.
Check if body is null, if it is, created default settings and return.
If it's not, try getting individual settings, the way it's already done, but without constant and the same null checks + syntax will be way shorter once you have body,

typhoon41 · 2024-12-13T16:05:24Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_buttons-generator.scss

+
+@mixin generate-from($theme) {
+	$theming-primary: map.get($theme, 'general', 'colors', 'primary');
+	$theming-font-family: map.get($theme, 'general', 'fonts', 'buttons', 'family');


Another variable to isolate map.get($theme, 'general', 'fonts', 'buttons') would make other variables shorter and more readable.

typhoon41 · 2024-12-13T16:06:34Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_buttons-generator.scss

+	$theming-font-weight: map.get($theme, 'general', 'fonts', 'buttons', 'weight');
+	$theming-font-size: map.get($theme, 'general', 'fonts', 'buttons', 'size');
+
+	$primary-color: if($theming-primary == '' or $theming-primary == null, mat.get-color-from-palette(mat.$primary-palette, 500), $theming-primary);


@Cvetanovic please communicate with @jovana-marceta where she has found this shorter IF syntax for Scss because I was not able to find it the latest official docs (which doesn't mean it's not working).

typhoon41 · 2024-12-13T16:10:20Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_checkboxes-generator.scss

+
+	.enigmatry-redactor-content {
+
+		$checkbox-color: if($theming-accent-color == '' or $theming-accent-color == null,


I understood we need to check for empty string and null for variables coming from AM, but this is our theme which leads me to the fact something's maybe not right on your machine, @jovana-marceta .
@Cvetanovic please switch to this branch and try testing this if condition without empty string and communicate with Jovana (and me eventually) whether is working and what exactly isn't since we expect null value if unset and not an empty string.

typhoon41 · 2024-12-13T16:56:47Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_tables-generator.scss

+
+		table, table td, table th {
+			/* stylelint-disable-next-line scss/at-if-no-null */
+			@if $theming-fonts and map.has-key($theming-fonts, 'body') and map.get($theming-fonts, 'body', 'size') != null {


Still legit.

typhoon41 · 2024-12-13T17:00:39Z

libs/entry-components/styles/modules/vendors/rich-text/necessary-mix/_buttons-generator.scss

+	.enigmatry-redactor-content {
+		button, .rx-content button {
+
+			@if (theming-buttons, 'colors', 'primary') {


That's fine. Short and sweat, but where did you find it? Nothing about that in official docs AFAIK?

BP-1293: Adding Redactor overrides in entry

94a3604

jovana-marceta requested review from typhoon41, Cvetanovic and msljivic October 30, 2024 08:53

jovana-marceta added 2 commits October 30, 2024 16:47

BP-1293: Extending redactor generator mixins

19895ee

BP-1293: Add redactor generators for buttons and table styles

1196aa4

jovana-marceta commented Nov 5, 2024

View reviewed changes

libs/entry-components/styles/modules/vendors/redactor/_buttons-generator.scss Outdated Show resolved Hide resolved

jovana-marceta commented Nov 5, 2024

View reviewed changes

libs/entry-components/styles/modules/vendors/redactor/_buttons-generator.scss Outdated Show resolved Hide resolved

jovana-marceta added 2 commits November 5, 2024 17:08

BP-1293: Adding redactor checkboxes generator

162bab4

BP-1293: Refactoring redactor styles

0444e4e

jovana-marceta commented Nov 6, 2024

View reviewed changes

libs/entry-components/styles/partials/theming.scss Show resolved Hide resolved

Use mixin for row coloring from foundation

71f0a1c

jovana-marceta commented Nov 6, 2024

View reviewed changes

libs/entry-components/styles/modules/vendors/redactor/_tables-generator.scss Outdated Show resolved Hide resolved

typhoon41 requested changes Nov 7, 2024

View reviewed changes

Cvetanovic reviewed Nov 7, 2024

View reviewed changes

libs/entry-components/styles/modules/vendors/redactor/_tables-generator.scss Outdated Show resolved Hide resolved

Cvetanovic reviewed Nov 7, 2024

View reviewed changes

libs/entry-components/styles/partials/vendors/overrides/redactor/_general.scss Outdated Show resolved Hide resolved

Jovana Marceta and others added 9 commits November 8, 2024 15:36

BP-1293: Inherit angular material checkbox background and change alia…

6f32991

…s for rich text

Rename redactor lib

d58d056

Move non-theming related variables outside of redactor mixins

22133a9

BP-1293: Add redactor layout generator and clean up generator file

dead72d

BP-1293: Inherit table even background from angular material

6768b1a

BP-1293: Inherit material body text color

80792fb

BP-1293: Make redactor typography generator more redeable

f4151f9

BP-1293: Rename folder

b9f241c

BP-1293: Resolve review suggestions

8d2d02d

jovana-marceta commented Nov 14, 2024

View reviewed changes

libs/entry-components/styles/partials/vendors/overrides/rich-text/_buttons.scss Show resolved Hide resolved

jovana-marceta added 2 commits November 14, 2024 11:59

BP-1293: Get rid of variables

5a54b11

rename checkbox variables

194e84e

jovana-marceta commented Nov 14, 2024

View reviewed changes

libs/entry-components/styles/partials/vendors/overrides/rich-text/_forms.scss Show resolved Hide resolved

jovana-marceta requested a review from Cvetanovic November 14, 2024 11:52

jovana-marceta requested a review from typhoon41 November 14, 2024 11:52

jovana-marceta commented Nov 14, 2024

View reviewed changes

rename toolbar generator

2bdd9ea

typhoon41 requested changes Nov 15, 2024

View reviewed changes

jovana-marceta added 7 commits November 20, 2024 12:48

BP-1293: Add missing sign on buttons generator variable

6977b0b

BP-1293: Get rid of checkboxes duplicated extraction

d4dda13

Get rid of toolbar generator duplicated extraction retrieval

0789b8f

Additional buttons and checkboxes file refactoring

608e968

Make buttons generator more redeable

b4ea198

Refactoring checkbox and toolbar generator

2bb570a

Fixed indentation for buttons generator

2102dcc

typhoon41 requested changes Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BP-1293: Add theming support to new rich-text editor for basic redactor configuration #240

BP-1293: Add theming support to new rich-text editor for basic redactor configuration #240

jovana-marceta commented Oct 30, 2024 •

edited

Loading

jovana-marceta Nov 14, 2024

typhoon41 Nov 15, 2024

typhoon41 Nov 15, 2024

jovana-marceta Nov 20, 2024

typhoon41 Dec 13, 2024

jovana-marceta Dec 26, 2024

typhoon41 Dec 27, 2024

typhoon41 Nov 15, 2024

typhoon41 Nov 15, 2024

typhoon41 Nov 15, 2024

typhoon41 Dec 13, 2024

typhoon41 Nov 15, 2024

typhoon41 Nov 15, 2024

typhoon41 Nov 15, 2024

typhoon41 Dec 13, 2024

typhoon41 Dec 13, 2024

typhoon41 Dec 13, 2024

typhoon41 Dec 13, 2024

typhoon41 Dec 13, 2024


		.enigmatry-redactor-content {

		$checkbox-color: if($theming-accent-color == '' or $theming-accent-color == null,

BP-1293: Add theming support to new rich-text editor for basic redactor configuration #240

Are you sure you want to change the base?

BP-1293: Add theming support to new rich-text editor for basic redactor configuration #240

Conversation

jovana-marceta commented Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jovana-marceta commented Oct 30, 2024 •

edited

Loading