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

EZP-31295: Unable to use default empty custom CSS class in the OE #1194

Merged
merged 2 commits into from
Jan 21, 2020

Conversation

SerheyDolgushev
Copy link
Contributor

@SerheyDolgushev SerheyDolgushev commented Jan 15, 2020

Question Answer
Tickets EZP-31295
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

In some cases custom CSS class is optional, and it should be possible to not select any. Right now UI is not allowing to do that. As a workaround, we added an empty custom CSS class and made it default, but it was causing some errors.

Steps to reproduce

  1. Setup custom CSS classed and default empty CSS class:
    system:
        default:
            fieldtypes:
                ezrichtext:
                    classes:
                        paragraph:
                            choices: ['', custom-paragraph-1, custom-paragraph-2, custom-paragraph-3]
                            default_value: ''
                            required: false
                            multiple: false
    
  2. Open the Online Editor and try to update the custom CSS class for the paragraph.

Actual result

Following JavaScript error is thrown:

Uncaught DOMException: Failed to execute 'remove' on 'DOMTokenList': The token provided must not be empty.
    at ButtonAttributesUpdate.clearClasses
    ...

Expected result

Custom CSS class should be changed, and it should be possible to change it to the empty default value at any given time.

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

Copy link
Member

@dew326 dew326 left a comment

Choose a reason for hiding this comment

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

Thanks for another contribution.
Looks ok, just minor code style.

@@ -143,7 +143,12 @@ export default class EzBtnAttributesUpdate extends EzWidgetButton {
return;
}

block.$.classList.remove(...this.classes.choices);
const classList = block.$.classList;
this.classes.choices.forEach(className => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.classes.choices.forEach(className => {
this.classes.choices.forEach((className) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dew326 done in 0ecdbd0

@dew326
Copy link
Member

dew326 commented Jan 17, 2020

@SerheyDolgushev could you prepare PR for ezplatform-richtext to fix it in 3.0?

@micszo micszo self-assigned this Jan 21, 2020
@dew326
Copy link
Member

dew326 commented Jan 21, 2020

PR for master created: ezsystems/ezplatform-richtext#104

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

QA Approved on eZ Platform EE v2.5.8 with diff.

@micszo micszo removed their assignment Jan 21, 2020
@SerheyDolgushev
Copy link
Contributor Author

@dew326 sorry, I was out for a few days. And seems like you just made a PR for master.

@lserwatka lserwatka merged commit 6905f78 into ezsystems:1.5 Jan 21, 2020
@lserwatka
Copy link
Member

Could you merge it up?

@dew326
Copy link
Member

dew326 commented Jan 22, 2020

@lserwatka done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants