[Feature suggestion] optionally insert styles from StyleManager with !important

[tommedema]

I'd like to suggest a feature that can be enabled through config: styleAsImportant: true (defaults to false, i.e. non-breaking). I thought this was already possible by configuring the cssComposer, though I've now learned that this is not the case (see below for an explanation).

I am working with templates that sometimes were generated from pre-compilers that excessively use the !important flag. For example a button in a template might be styled as:

._style_wxcCz {
  border-style: solid !important;
  margin: 0 !important;
  padding-right: 20px !important;
  padding-top: 11px !important;
  padding-bottom: 11px !important;
  background-color: #377037 !important;
  font-family: ff-clan-web-pro, "Helvetica Neue", Helvetica, sans-serif !important;
  font-weight: 600 !important;
  border-color: #377037 !important;
  border-radius: 0 !important;
  padding-left: 20px !important;
  border-width: 2px !important;
  color: #ffffff !important;
  cursor: pointer !important;
  display: inline-block !important;
  font-size: 14px !important;
  line-height: 18px !important;
  outline: none !important;
  overflow: visible !important;
  position: relative !important;
  text-align: center !important;
  text-decoration: none !important;
  text-transform: uppercase !important;
  vertical-align: middle !important
}

As a result, styling this button with Grapes has absolutely no effect. The most obvious way to make it work with grapes, is to ensure that:

1. the grapes styling also has !important (this feature)
2. the grapes styling comes after the template's styling (should already be the case, right?)

I guessed that the proper way would be to make CssRule.js#36 configureable. I tried to set this important flag using:

var editor = grapesjs.init(
        {
            cssComposer: { important: true },
        ... 
        }
);

But this does not seem to have an effect. Moreover, when I hardcode it to true in the code, I see that there are two issues with this implementation:

1. it also adds !important to rules that come from the template. This feature suggestion is specifically for rules that are manually added through the style manager, because these should overwrite the template's styles. Otherwise there is no added value.
2. it adds !important twice if there is already an important statement in rules from the template, causing the rendering to fail due to having invalid css properties:

Therefore my suggestion is to create a new config value styleAsImportant: true (defaults to false, i.e. non-breaking). After some initial debugging I found that this change makes this happen:

style_manager/index.js line 283:

from:

rule = cssC.add(valid, state, deviceW);

to:

rule = cssC.add(valid, state, deviceW, { important: config.styleAsImportant });

This seems to work well. It does not add !important to rules from the template, and it does add it to rules set by the style manager. This is the desired behavior.

Do you agree? Should I submit a PR?

Cheers

[chapterjason]

Yeah, there should be a way to solve this. But I have to say that using !important is a really bad practice.

I would rather do something like strip the important statements out to get this working.

Could I know what kind of pre-compiler you're using? Looks really wired...

[tommedema]

@chapterjason I am not in control over the template. So I wouldn't know how to answer that question. Grapes should work with any HTML and CSS, so I don't think it's relevant.

I would not want to remove !important statements from templates, because doing so can affect the render. If they have multiple statements, some with important and some without, and in a certain order, you want it to render exactly as it was intended to. Removing important statements would change this.

[artf]

It might be a good idea, but what about improving those events (eg. pass model/pr to callbacks) to allow such customizations

[tommedema]

@artf do you mean that we'd have to then write plugins to listen to those events and add additional styling to the component? This seems complicated as it depends on if the styling is inline or through classes.

Since the core of grapes is editing and styling components, I'd argue that this feature suits well in core. There is already an important flag anyway, it just doesn't work as one would expect :)

[artf]

@tommedema Components and CssRules implement Styleable, therefore you would get it in all case.

do you mean that we'd have to then write plugins to listen to those events and add additional styling to the component?

Not additional, you overwrite the style triggered on that model, eg.

	// eg. your configs
	const styleImportant = 1;
	// ...
    editor.on('styleable:change', (model, property) => {
      const value = model.getStyle()[property];
      console.log('Styled ', property, value);
      if (styleImportant) {
        model.addStyle({ [property]: value + ` !important` });
      }
    });

[tommedema]

added PR at #1056

[tommedema]

@artf so now that I am using this in production it turns out that there is an issue with the style manager's color picker when styles have !important:

(e50000 is red but it's shown as white here)

The color picker becomes buggy in several ways, probably because it cannot parse the color anymore after !important is added.

1. when choosing a color and pressing "OK" in the color picker, the new color is ignored and the old color is kept
2. when choosing a color and then dismissing the color picker by pressing outside of its view, the new color is kept; but when you then view the color picker again, it does not show the correct color preview due to the important statement confusing it

The important statement should probably be ignored by the color picker, before it tries to parse the color?

[artf]

The important statement should probably be ignored by the color picker, before it tries to parse the color?

To be honest, I'd expect such a check on all inputs 🤔 Do have such an issue only with color inputs?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.