From 3f5056d8fe5e417c121d8ee0d92daeeaaca40766 Mon Sep 17 00:00:00 2001 From: devDobby Date: Tue, 11 Jun 2024 11:07:39 +0200 Subject: [PATCH 1/4] Show style applied on tag (p, span, div) and private selectors as parent rules instead of hiding them (#5890) * GrapesJS can now returns multiple CSS rules class as parents and CSS rule applied to tag name * Take last version of grape. Fix bug with private selectors that stay invisible in style manager when another class was applied. * Fix reviews; * Simplify code to work only with single tag name; --------- Co-authored-by: Artur Arseniev --- src/style_manager/index.ts | 32 +++++++++++++-- test/specs/style_manager/index.ts | 66 +++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/src/style_manager/index.ts b/src/style_manager/index.ts index 196c20944..38044baca 100644 --- a/src/style_manager/index.ts +++ b/src/style_manager/index.ts @@ -597,6 +597,8 @@ export default class StyleManager extends ItemManagerModule< const cmp = target.toHTML ? target : target.getComponent(); const optsSel = { array: true } as const; let cmpRules: CssRule[] = []; + let tagNameRules: CssRule[] = []; + let invisibleAndOtherRules: CssRule[] = []; let otherRules: CssRule[] = []; let rules: CssRule[] = []; @@ -605,19 +607,41 @@ export default class StyleManager extends ItemManagerModule< ? [] : cssC.getRules().filter(rule => { const rSels = rule.getSelectors().map(s => s.getFullName()); - return !!rSels.length && rSels.every(rSel => values.indexOf(rSel) >= 0); + + // rSels is equal to 0 when rule selectors contain tagName like : p {}, .logo path {}, ul li {} + if (rSels.length === 0) { + return false; + } + + return rSels.every(rSel => values.indexOf(rSel) >= 0); }); }; + const rulesByTagName = (tagName: string) => { + return !tagName ? [] : cssC.getRules().filter(rule => rule.selectorsToString() === tagName); + }; + // Componente related rule if (cmp) { cmpRules = cssC.getRules(`#${cmp.getId()}`); + tagNameRules = rulesByTagName(cmp.get('tagName')); otherRules = sel ? rulesBySelectors(sel.getSelectors().getFullName(optsSel)) : []; - rules = otherRules.concat(cmpRules); + rules = otherRules.concat(tagNameRules).concat(cmpRules); } else { cmpRules = sel ? cssC.getRules(`#${sel.getId()}`) : []; - otherRules = rulesBySelectors(target.getSelectors().getFullName(optsSel)); - rules = cmpRules.concat(otherRules); + tagNameRules = rulesByTagName(sel?.get('tagName') || ''); + // Get rules set on invisible selectors like private one + const allCmpClasses = sel?.getSelectors().getFullName(optsSel) || []; + const invisibleSel = allCmpClasses.filter( + (item: string) => + target + .getSelectors() + .getFullName(optsSel) + .findIndex(sel => sel === item) === -1 + ); + // Get rules set on active and visible selectors + invisibleAndOtherRules = rulesBySelectors(invisibleSel.concat(target.getSelectors().getFullName(optsSel))); + rules = tagNameRules.concat(cmpRules).concat(invisibleAndOtherRules); } const all = rules diff --git a/test/specs/style_manager/index.ts b/test/specs/style_manager/index.ts index d3d8c9c08..9eb0bf937 100644 --- a/test/specs/style_manager/index.ts +++ b/test/specs/style_manager/index.ts @@ -195,6 +195,72 @@ describe('StyleManager', () => { expect(obj.getSelectedParents()).toEqual([rule1]); }); + test('With multiple classes, should both be in parents list', () => { + const cmp = domc.addComponent('
'); + const [rule1, rule2] = cssc.addRules(` + .cls { color: red; } + .cls2 { color: blue; } + `); + em.setSelected(cmp); + obj.__upSel(); + expect(obj.getSelected()).not.toBe(rule1); + expect(obj.getSelected()).not.toBe(rule2); + expect(obj.getSelectedParents()).toEqual([rule2, rule1]); + }); + + test('With tagName + class, class first', () => { + const cmp = domc.addComponent('
'); + const [rule1, rule2] = cssc.addRules(` + .cls { color: red; } + div { color: yellow; } + `); + em.setSelected(cmp); + obj.__upSel(); + expect(obj.getSelected()).toBe(rule1); + expect(obj.getSelectedParents()).toEqual([rule2]); + }); + + test('Should ignore rules with tagName in the selector path but the rule is not apply on the tagName', () => { + const cmp = domc.addComponent('
'); + const [rule1, rule2] = cssc.addRules(` + .cls { color: red; } + div { color: yellow; } + div .child { padding: 10px; } + `); + em.setSelected(cmp); + obj.__upSel(); + // getSelectedParents should only have 1 rule as the third one is not applied on the div + expect(obj.getSelected()).toBe(rule1); + expect(obj.getSelectedParents()).toEqual([rule2]); + }); + + test('Should tagName rules if the selectors does not contain only the tagNale', () => { + const cmp = domc.addComponent('
'); + const [rule1, rule2] = cssc.addRules(` + .cls { color: red; } + div { color: yellow; } + .child div { padding: 10px; } + `); + em.setSelected(cmp); + obj.__upSel(); + // getSelectedParents should only have 1 rule as the third one is not applied on the div + expect(obj.getSelected()).toBe(rule1); + expect(obj.getSelectedParents()).toEqual([rule2]); + }); + + test('With tagName + ID + class, class first, ID second', () => { + const cmp = domc.addComponent('
'); + const [rule1, rule2, rule3] = cssc.addRules(` + .cls { color: red; } + div { color: yellow; } + #id-test { color: blue; } + `); + em.setSelected(cmp); + obj.__upSel(); + expect(obj.getSelected()).toBe(rule1); + expect(obj.getSelectedParents()).toEqual([rule3, rule2]); + }); + test('With ID + class, multiple devices', () => { sm.setComponentFirst(true); const cmp = domc.addComponent('
'); From 509e410901a4013eea3046799c5f37232cfc072e Mon Sep 17 00:00:00 2001 From: devDobby Date: Sun, 16 Jun 2024 12:34:09 +0200 Subject: [PATCH 2/4] Add an option to return inline style even if we want to avoid it at the beginning (#5933) --- src/common/index.ts | 2 +- src/dom_components/model/Component.ts | 16 ++++++++----- src/dom_components/model/types.ts | 5 ++++ src/editor/config/config.ts | 6 +++++ test/specs/dom_components/model/Component.ts | 24 ++++++++++++++++---- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/common/index.ts b/src/common/index.ts index 57c000ce6..7adcc4978 100644 --- a/src/common/index.ts +++ b/src/common/index.ts @@ -5,7 +5,7 @@ interface NOOP {} export type Debounced = Function & { cancel(): void }; -export type SetOptions = Backbone.ModelSetOptions & { avoidStore?: boolean }; +export type SetOptions = Backbone.ModelSetOptions & { avoidStore?: boolean; inline?: boolean }; export type AddOptions = Backbone.AddOptions & { temporary?: boolean; action?: string }; diff --git a/src/dom_components/model/Component.ts b/src/dom_components/model/Component.ts index e642c6374..2edff81cb 100644 --- a/src/dom_components/model/Component.ts +++ b/src/dom_components/model/Component.ts @@ -550,7 +550,7 @@ export default class Component extends StyleableModel { // Handle style const style = attrs.style; - style && this.setStyle(style); + style && this.setStyle(style, opts); delete attrs.style; const attrPrev = { ...this.previous('attributes') }; @@ -627,6 +627,10 @@ export default class Component extends StyleableModel { if (rule) { return rule.getStyle(prop); } + + // Return empty style if not rule have been found. We cannot return inline style with the next return + // because else on load inline style is set a #id or .class style + return {}; } return super.getStyle.call(this, prop); @@ -685,9 +689,9 @@ export default class Component extends StyleableModel { // Add style if (!opts.noStyle) { - const style = this.get('style'); + const style = this.getStyle({ inline: true }); if (isObject(style) && !isEmptyObj(style)) { - attributes.style = this.styleToString({ inline: 1 }); + attributes.style = this.styleToString({ inline: true }); } } @@ -1589,7 +1593,7 @@ export default class Component extends StyleableModel { const tag = customTag || model.get('tagName'); const sTag = model.get('void'); const customAttr = opts.attributes; - let attributes = this.getAttrToHTML(); + let attributes = this.getAttrToHTML(opts); delete opts.tag; // Get custom attributes if requested @@ -1660,10 +1664,10 @@ export default class Component extends StyleableModel { * @return {Object} * @private */ - getAttrToHTML() { + getAttrToHTML(opts?: ToHTMLOptions) { const attrs = this.getAttributes(); - if (avoidInline(this.em)) { + if (avoidInline(this.em) && opts?.keepInlineStyle !== true) { delete attrs.style; } diff --git a/src/dom_components/model/types.ts b/src/dom_components/model/types.ts index 4a94f52d1..860d886f3 100644 --- a/src/dom_components/model/types.ts +++ b/src/dom_components/model/types.ts @@ -288,6 +288,11 @@ export interface ToHTMLOptions extends OptionAsDocument { */ altQuoteAttr?: boolean; + /** + * Keep inline style set intentionally by users with `setStyle({}, { inline: true })` + */ + keepInlineStyle?: boolean; + /** * You can pass an object of custom attributes to replace with the current ones * or you can even pass a function to generate attributes dynamically. diff --git a/src/editor/config/config.ts b/src/editor/config/config.ts index da717a880..a95d84c83 100644 --- a/src/editor/config/config.ts +++ b/src/editor/config/config.ts @@ -7,6 +7,7 @@ import { DeviceManagerConfig } from '../../device_manager/config/config'; import { I18nConfig } from '../../i18n/config'; import { ModalConfig } from '../../modal_dialog/config/config'; import { LayerManagerConfig } from '../../navigator/config/config'; +import { KeymapsConfig } from '../../keymaps/config'; import { PageManagerConfig } from '../../pages/types'; import { PanelsConfig } from '../../panels/config/config'; import { ParserConfig } from '../../parser/config/config'; @@ -356,6 +357,11 @@ export interface EditorConfig { */ commands?: CommandsConfig; + /** + * Configurations for keymaps + */ + keymaps?: KeymapsConfig; + /** * Configurations for Css Composer. */ diff --git a/test/specs/dom_components/model/Component.ts b/test/specs/dom_components/model/Component.ts index 1d6b27e6b..6a966ad11 100644 --- a/test/specs/dom_components/model/Component.ts +++ b/test/specs/dom_components/model/Component.ts @@ -19,7 +19,9 @@ let em: Editor; describe('Component', () => { beforeEach(() => { - em = new Editor({ avoidDefaults: true }); + // FIXME: avoidInlineStyle is deprecated and when running in dev or prod, `avoidInlineStyle` is set to true + // The following tests ran with `avoidInlineStyle` to false (this is why I add the parameter here) + em = new Editor({ avoidDefaults: true, avoidInlineStyle: true }); dcomp = em.Components; em.Pages.onLoad(); compOpts = { @@ -278,10 +280,10 @@ describe('Component', () => { class: 'class1 class2', style: 'color: white; background: #fff', }); + // Style is not in attributes because it has not been set as inline expect(obj.getAttributes()).toEqual({ id: 'test', class: 'class1 class2', - style: 'color:white;background:#fff;', 'data-test': 'value', }); expect(obj.classes.length).toEqual(2); @@ -291,13 +293,25 @@ describe('Component', () => { }); }); - test('set inline style with multiple values of the same key', () => { + test('set style with multiple values of the same key', () => { obj.setAttributes({ style: CSS_BG_STR }); expect(obj.getStyle()).toEqual(CSS_BG_OBJ); }); - test('get proper style from inline style with multiple values of the same key', () => { - obj.setAttributes({ style: CSS_BG_STR }); + test('set style on id and inline style', () => { + obj.setStyle({ color: 'red' }); // Should be set on id + obj.setStyle({ display: 'flex' }, { inline: true }); // Should be set as inline + + expect(obj.getStyle()).toEqual({ + color: 'red', + }); + expect(obj.getStyle({ inline: true })).toEqual({ + display: 'flex', + }); + }); + + test('get proper style from style with multiple values of the same key', () => { + obj.setAttributes({ style: CSS_BG_STR }, { inline: true }); expect(obj.getAttributes()).toEqual({ style: CSS_BG_STR.split('\n').join(''), }); From a62f92739482ce94823eae52aeb2cc5e1c1207fd Mon Sep 17 00:00:00 2001 From: Artur Arseniev Date: Sun, 16 Jun 2024 14:52:47 +0400 Subject: [PATCH 3/4] Update TS --- src/common/index.ts | 2 +- src/css_composer/index.ts | 5 +++-- src/dom_components/model/Component.ts | 13 +++++++------ src/domain_abstract/model/StyleableModel.ts | 6 ++++-- src/navigator/index.ts | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/common/index.ts b/src/common/index.ts index 7adcc4978..57c000ce6 100644 --- a/src/common/index.ts +++ b/src/common/index.ts @@ -5,7 +5,7 @@ interface NOOP {} export type Debounced = Function & { cancel(): void }; -export type SetOptions = Backbone.ModelSetOptions & { avoidStore?: boolean; inline?: boolean }; +export type SetOptions = Backbone.ModelSetOptions & { avoidStore?: boolean }; export type AddOptions = Backbone.AddOptions & { temporary?: boolean; action?: string }; diff --git a/src/css_composer/index.ts b/src/css_composer/index.ts index f09134030..e6a866a9e 100644 --- a/src/css_composer/index.ts +++ b/src/css_composer/index.ts @@ -39,6 +39,7 @@ import { ItemManagerModule } from '../abstract/Module'; import EditorModel from '../editor/model/Editor'; import Component from '../dom_components/model/Component'; import { ObjectAny, PrevToNewIdMap } from '../common'; +import { UpdateStyleOptions } from '../domain_abstract/model/StyleableModel'; /** @private */ interface RuleOptions { @@ -53,7 +54,7 @@ interface RuleOptions { } /** @private */ -interface SetRuleOptions extends RuleOptions { +interface SetRuleOptions extends RuleOptions, UpdateStyleOptions { /** * If the rule exists already, merge passed styles instead of replacing them. */ @@ -61,7 +62,7 @@ interface SetRuleOptions extends RuleOptions { } /** @private */ -export interface GetSetRuleOptions { +export interface GetSetRuleOptions extends UpdateStyleOptions { state?: string; mediaText?: string; addOpts?: ObjectAny; diff --git a/src/dom_components/model/Component.ts b/src/dom_components/model/Component.ts index 29810df29..aca0c7d94 100644 --- a/src/dom_components/model/Component.ts +++ b/src/dom_components/model/Component.ts @@ -12,7 +12,7 @@ import { keys, } from 'underscore'; import { shallowDiff, capitalize, isEmptyObj, isObject, toLowerCase } from '../../utils/mixins'; -import StyleableModel, { StyleProps } from '../../domain_abstract/model/StyleableModel'; +import StyleableModel, { StyleProps, UpdateStyleOptions } from '../../domain_abstract/model/StyleableModel'; import { Model } from 'backbone'; import Components from './Components'; import Selector from '../../selector_manager/model/Selector'; @@ -22,7 +22,6 @@ import EditorModel from '../../editor/model/Editor'; import { AddComponentsOption, ComponentAdd, - ComponentAddType, ComponentDefinition, ComponentDefinitionDefined, ComponentOptions, @@ -44,6 +43,8 @@ import ItemView from '../../navigator/view/ItemView'; export interface IComponent extends ExtractMethods {} +export interface SetAttrOptions extends SetOptions, UpdateStyleOptions {} + const escapeRegExp = (str: string) => { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&'); }; @@ -541,7 +542,7 @@ export default class Component extends StyleableModel { * Emit changes for each updated attribute * @private */ - attrUpdated(m: any, v: any, opts: any = {}) { + attrUpdated(m: any, v: any, opts: SetAttrOptions = {}) { const attrs = this.get('attributes')!; // Handle classes const classes = attrs.class; @@ -570,7 +571,7 @@ export default class Component extends StyleableModel { * @example * component.setAttributes({ id: 'test', 'data-key': 'value' }); */ - setAttributes(attrs: ObjectAny, opts: SetOptions = {}) { + setAttributes(attrs: ObjectAny, opts: SetAttrOptions = {}) { this.set('attributes', { ...attrs }, opts); return this; } @@ -583,7 +584,7 @@ export default class Component extends StyleableModel { * @example * component.addAttributes({ 'data-key': 'value' }); */ - addAttributes(attrs: ObjectAny, opts: SetOptions = {}) { + addAttributes(attrs: ObjectAny, opts: SetAttrOptions = {}) { return this.setAttributes( { ...this.getAttributes({ noClass: true }), @@ -643,7 +644,7 @@ export default class Component extends StyleableModel { * @example * component.setStyle({ color: 'red' }); */ - setStyle(prop: StyleProps = {}, opts: any = {}) { + setStyle(prop: StyleProps = {}, opts: UpdateStyleOptions = {}) { const { opt, em } = this; if (avoidInline(em) && !opt.temporary && !opts.inline) { diff --git a/src/domain_abstract/model/StyleableModel.ts b/src/domain_abstract/model/StyleableModel.ts index a6b450860..811c8466f 100644 --- a/src/domain_abstract/model/StyleableModel.ts +++ b/src/domain_abstract/model/StyleableModel.ts @@ -1,14 +1,16 @@ import { isArray, isString, keys } from 'underscore'; -import { Model, ObjectAny, ObjectHash } from '../../common'; +import { Model, ObjectAny, ObjectHash, SetOptions } from '../../common'; import ParserHtml from '../../parser/model/ParserHtml'; import Selectors from '../../selector_manager/model/Selectors'; import { shallowDiff } from '../../utils/mixins'; export type StyleProps = Record; -export type UpdateStyleOptions = ObjectAny & { +export type UpdateStyleOptions = SetOptions & { partial?: boolean; addStyle?: StyleProps; + inline?: boolean; + noEvent?: boolean; }; const parserHtml = ParserHtml(); diff --git a/src/navigator/index.ts b/src/navigator/index.ts index 13d2c428b..76c7ff471 100644 --- a/src/navigator/index.ts +++ b/src/navigator/index.ts @@ -195,7 +195,7 @@ export default class LayerManager extends Module { style.display = 'none'; } - component.setStyle(style, styleOpts); + component.setStyle(style, styleOpts as any); this.updateLayer(component); this.em.trigger('component:toggled'); // Updates Style Manager #2938 } From f20d7d3b18f92daba16bf18ed18a11f57f38ed1d Mon Sep 17 00:00:00 2001 From: Artur Arseniev Date: Sun, 16 Jun 2024 16:14:37 +0400 Subject: [PATCH 4/4] Toggle the move icon in Layers when component `draggable` changes. --- src/navigator/view/ItemView.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/navigator/view/ItemView.ts b/src/navigator/view/ItemView.ts index c1b1cd7fa..a2e97bd81 100644 --- a/src/navigator/view/ItemView.ts +++ b/src/navigator/view/ItemView.ts @@ -20,12 +20,13 @@ export type ItemViewProps = ViewOptions & { }; const inputProp = 'contentEditable'; +const dataToggleMove = 'data-toggle-move'; export default class ItemView extends View { events() { return { - 'mousedown [data-toggle-move]': 'startSort', - 'touchstart [data-toggle-move]': 'startSort', + [`mousedown [${dataToggleMove}]`]: 'startSort', + [`touchstart [${dataToggleMove}]`]: 'startSort', 'click [data-toggle-visible]': 'toggleVisibility', 'click [data-toggle-open]': 'toggleOpening', 'click [data-toggle-select]': 'handleSelect', @@ -76,7 +77,7 @@ export default class ItemView extends View {
${count || ''}
-
${move || ''}
+
${move || ''}
@@ -151,6 +152,7 @@ export default class ItemView extends View { ['change:open', this.updateOpening], ['change:layerable', this.updateLayerable], ['change:style:display', this.updateVisibility], + ['change:draggable', this.updateMove], ['rerender:layer', this.render], ['change:name change:custom-name', this.updateName], // @ts-ignore @@ -183,6 +185,15 @@ export default class ItemView extends View { this.getVisibilityEl()[method](`${pfx}layer-off`); } + updateMove() { + const { model, config } = this; + const el = this.getItemContainer().find(`[${dataToggleMove}]`)[0]; + if (el) { + const isDraggable = model.get('draggable') && config.sortable; + el.style.display = isDraggable ? '' : 'none'; + } + } + /** * Toggle visibility * @param Event @@ -421,16 +432,13 @@ export default class ItemView extends View { el.find(`.${this.clsChildren}`).append(children); } - if (!model.get('draggable') || !config.sortable) { - el.children(`.${this.clsMove}`).remove(); - } - !module.isVisible(model) && (this.className += ` ${pfx}hide`); hidden && (this.className += ` ${ppfx}hidden`); el.attr('class', this.className!); this.updateStatus(); this.updateOpening(); this.updateVisibility(); + this.updateMove(); this.__render(); this._rendered = true; return this;