Skip to content
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

feat: Add css classes from json block definitions #8377

Conversation

ishon19
Copy link

@ishon19 ishon19 commented Jul 16, 2024

The basics

The details

This PR overrides the jsonInit method in the BlockSvg class to get the css class names from the classes field and apply the same using the addClass method.

Fixes #8271

@ishon19 ishon19 requested a review from a team as a code owner July 16, 2024 12:44
@ishon19 ishon19 requested a review from BeksOmega July 16, 2024 12:44
@github-actions github-actions bot added the PR: fix Fixes a bug label Jul 16, 2024
Comment on lines 1728 to 1736
let classesToAdd = '';

if (Array.isArray(json['classes'])) {
classesToAdd = json['classes'].join(' ');
} else {
classesToAdd = json['classes'];
}

this.addClass(classesToAdd);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simplify this using a ternary operator.

Suggested change
let classesToAdd = '';
if (Array.isArray(json['classes'])) {
classesToAdd = json['classes'].join(' ');
} else {
classesToAdd = json['classes'];
}
this.addClass(classesToAdd);
this.addClass(Array.isArray(json['classes']) ? json['classes'].join(' ') : json['classes']);

You'll probably need to run npm run format in the root directory after applying this change to handle the line wrapping.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, thanks for pointing that out! Updated that and formatted using npm run format.

@ishon19 ishon19 requested a review from BeksOmega July 16, 2024 18:16
Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweetness! Thank you for the fix this looks great =) I'll get this merged once CI passes.

@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Jul 16, 2024
@BeksOmega BeksOmega changed the title fix: Add css classes from json block definitions feat: Add css classes from json block definitions Jul 17, 2024
@github-actions github-actions bot added PR: feature Adds a feature and removed PR: fix Fixes a bug labels Jul 17, 2024
@BeksOmega BeksOmega merged commit a2a5749 into google:rc/v12.0.0 Jul 17, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants