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

Refactor Traits Collection #4983

Merged
merged 4 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,11 @@ insert_final_newline = true
charset = utf-8
indent_style = space
indent_size = 2
quote_type = single

[*.ts]
charset = utf-8
indent_style = space
indent_size = 2
quote_type = single
max_line_length = 120
7 changes: 2 additions & 5 deletions src/trait_manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const evAll = 'trait';
export const evPfx = `${evAll}:`;
export const evCustom = `${evPfx}custom`;

const typesDef = {
const typesDef: { [id: string]: { new (o: any): TraitView } } = {
text: TraitView,
number: TraitNumberView,
select: TraitSelectView,
Expand Down Expand Up @@ -62,12 +62,9 @@ export default class TraitManager extends Module<TraitManagerConfig & { pStylePr
*/
constructor(em: EditorModel) {
super(em, 'TraitManager', defaults);
const c = this.config;
const model = new Model();
this.model = model;
const ppfx = c.pStylePrefix;
this.types = { ...typesDef };
ppfx && (c.stylePrefix = `${ppfx}${c.stylePrefix}`);
this.types = typesDef;
xQwexx marked this conversation as resolved.
Show resolved Hide resolved

const upAll = debounce(() => this.__upSel(), 0);
model.listenTo(em, 'component:toggled', upAll);
Expand Down
13 changes: 10 additions & 3 deletions src/trait_manager/model/Trait.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export interface TraitProperties {
*/
export default class Trait extends Model<TraitProperties> {
target!: Component;
em?: EditorModel;
em: EditorModel;
view?: TraitView;
el?: HTMLElement;

Expand All @@ -83,12 +83,19 @@ export default class Trait extends Model<TraitProperties> {
};
}

constructor(prop: TraitProperties) {
constructor(prop: TraitProperties, em: EditorModel) {
super(prop);
const { target, name, changeProp, value: initValue } = this.attributes;
const { target, name } = this.attributes;
!this.get('id') && this.set('id', name);
if (target) {
this.setTarget(target);
}
this.em = em;
}

setTarget(target: Component) {
if (target) {
const { name, changeProp, value: initValue } = this.attributes;
this.target = target;
this.unset('target');
const targetEvent = changeProp ? `change:${name}` : `change:attributes:${name}`;
Expand Down
54 changes: 27 additions & 27 deletions src/trait_manager/model/TraitFactory.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
import { TraitManagerConfig } from '../config/config';
import { TraitProperties } from './Trait';
import { isString } from 'underscore';
import Trait, { TraitProperties } from './Trait';
import EditorModel from '../../editor/model/Editor';

export default class TraitFactory {
config: Partial<TraitManagerConfig>;

constructor(config: Partial<TraitManagerConfig> = {}) {
this.config = config;
}

export default (config: Partial<TraitManagerConfig> = {}) => ({
/**
* Build props object by their name
* @param {Array<string>|string} props Array of properties name
* @return {Array<Object>}
*/
build(props: string | string[]) {
const objs = [];

if (typeof props === 'string') props = [props];
build(prop: string | TraitProperties, em: EditorModel): Trait {
return isString(prop) ? this.buildFromString(prop, em) : new Trait(prop, em);
}

for (let i = 0; i < props.length; i++) {
const prop = props[i];
const obj: TraitProperties = {
name: prop,
type: 'text',
};
private buildFromString(name: string, em: EditorModel): Trait {
const obj: TraitProperties = {
name: name,
type: 'text',
};

switch (prop) {
case 'target':
obj.type = 'select';
obj.default = false;
obj.options = config.optionsTarget;
break;
}

objs.push(obj);
switch (name) {
case 'target':
obj.type = 'select';
obj.default = false;
obj.options = this.config.optionsTarget;
break;
}

return objs;
},
});
return new Trait(obj, em);
}
}
43 changes: 22 additions & 21 deletions src/trait_manager/model/Traits.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isString, isArray } from 'underscore';
import { isArray } from 'underscore';
import { AddOptions, Collection } from '../../common';
import Component from '../../dom_components/model/Component';
import EditorModel from '../../editor/model/Editor';
Expand All @@ -8,12 +8,16 @@ import TraitFactory from './TraitFactory';
export default class Traits extends Collection<Trait> {
em: EditorModel;
target!: Component;
tf: TraitFactory;

constructor(coll: TraitProperties[], options: { em: EditorModel }) {
super(coll);
this.em = options.em;
this.listenTo(this, 'add', this.handleAdd);
this.listenTo(this, 'reset', this.handleReset);
const tm = this.em?.Traits;
const tmOpts = tm?.getConfig();
this.tf = new TraitFactory(tmOpts);
}

handleReset(coll: TraitProperties[], { previousModels = [] }: { previousModels?: Trait[] } = {}) {
Expand All @@ -31,32 +35,29 @@ export default class Traits extends Collection<Trait> {

setTarget(target: Component) {
this.target = target;
this.models.forEach(trait => trait.setTarget(target));
}

/** @ts-ignore */
add(models: string | Trait | TraitProperties | (string | Trait | TraitProperties)[], opt?: AddOptions) {
const em = this.em;

// Use TraitFactory if necessary
if (isString(models) || isArray(models)) {
const tm = em && em.get! && em.Traits;
const tmOpts = tm && tm.getConfig();
const tf = TraitFactory(tmOpts);

if (isString(models)) {
models = [models];
}
add(model: string | TraitProperties | Trait, options?: AddOptions): Trait;
add(models: Array<string | TraitProperties | Trait>, options?: AddOptions): Trait[];
add(models: unknown, opt?: AddOptions): any {
if (models == undefined) {
return undefined;
}
const { target, em } = this;

if (isArray(models)) {
var traits: Trait[] = [];
for (var i = 0, len = models.length; i < len; i++) {
const str = models[i];
const model = isString(str) ? tf.build(str)[0] : str;
model.target = this.target;
models[i] = model as Trait;
const trait = models[i];
traits[i] = trait instanceof Trait ? trait : this.tf.build(trait, em);
traits[i].setTarget(target);
}
return super.add(traits, opt);
}
const trait = models instanceof Trait ? models : this.tf.build(models as any, em);
trait.setTarget(target);

return Collection.prototype.add.apply(this, [models as Trait[], opt]);
return super.add(trait, opt);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure exactly what is happening here but with these changes to Traits, undo/redo on traits doesn't work properly (eg. I tried my latest fix here and it works only if I rollback these changes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a test to cover the undo scenario and fixed it :) Sorry I couldn't have time earlier :(

}
}

Traits.prototype.model = Trait;
2 changes: 1 addition & 1 deletion src/trait_manager/view/TraitView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ export default class TraitView extends View<Trait> {
const { type } = model.attributes;
this.config = config;
this.em = config.em;
this.pfx = config.stylePrefix || '';
this.ppfx = config.pStylePrefix || '';
this.pfx = this.ppfx + config.stylePrefix || '';
this.target = target;
const { ppfx } = this;
this.clsField = `${ppfx}field ${ppfx}field-${type}`;
Expand Down
6 changes: 3 additions & 3 deletions src/trait_manager/view/TraitsView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ export default class TraitsView extends DomainViews {
super(o);
this.itemsView = itemsView;
const config = o.config || {};
const pfx = config.stylePrefix || '';

const em = o.editor;
this.config = config;
this.em = em;
this.pfx = pfx;
this.ppfx = config.pStylePrefix || '';
this.className = `${pfx}traits`;
this.pfx = this.ppfx + config.stylePrefix || '';
this.className = `${this.pfx}traits`;
this.listenTo(em, 'component:toggled', this.updatedCollection);
this.updatedCollection();
}
Expand Down
32 changes: 27 additions & 5 deletions test/specs/trait_manager/model/TraitsModel.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,44 @@
import Trait from '../../../../src/trait_manager/model/Trait';
import Traits from '../../../../src/trait_manager/model/Traits';
import Component from '../../../../src/dom_components/model/Component';
import Editor from '../../../../src/editor';
import EditorModel from '../../../../src/editor/model/Editor';

describe('TraitModels', () => {
var trait: Trait;
var target: Component;
var modelName = 'title';
var em: EditorModel;

beforeEach(() => {
target = new Component();
trait = new Trait({
name: modelName,
target,
});
em = new Editor().getModel();
target = new Component({}, { em });
trait = new Trait(
{
name: modelName,
target,
},
em
);
});

afterEach(() => {});

test('Object exists', () => {
expect(trait).toBeTruthy();
});
test('Traits undo property', () => {
em.loadOnStart();
const wrapper = em.Components.getWrapper();
wrapper!.append(target);
const traits = new Traits([], { em });
traits.add(modelName);
traits.setTarget(target);
const trait = traits.models[0];
trait.setTargetValue('TitleValue');

expect(target.getAttributes()[modelName]).toBe('TitleValue');
em.UndoManager.undo();
expect(target.getAttributes()[modelName]).toBeUndefined;
});
});
37 changes: 25 additions & 12 deletions test/specs/trait_manager/view/TraitsView.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import Trait from '../../../../src/trait_manager/model/Trait';
import TraitView from '../../../../src/trait_manager/view/TraitView';
import Component from '../../../../src/dom_components/model/Component';
import EditorModel from '../../../../src/editor/model/Editor';
import Editor from '../../../../src/editor';

describe('TraitView', () => {
var obj: TraitView;
var trait: Trait;
var modelName = 'title';
var target: Component;
var em: EditorModel;

beforeEach(() => {
em = new Editor().getModel();
target = new Component();
trait = new Trait({
name: modelName,
target,
});
trait = new Trait(
{
name: modelName,
target,
},
em
);
obj = new TraitView({
model: trait,
});
Expand All @@ -34,14 +41,20 @@ describe('TraitView', () => {
test('Updates on different models do not alter other targets', () => {
var target1 = new Component();
var target2 = new Component();
var trait1 = new Trait({
name: modelName,
target: target1,
});
var trait2 = new Trait({
name: modelName,
target: target2,
});
var trait1 = new Trait(
{
name: modelName,
target: target1,
},
em
);
var trait2 = new Trait(
{
name: modelName,
target: target2,
},
em
);
var obj1 = new TraitView({ model: trait1 });
var obj2 = new TraitView({ model: trait2 });

Expand Down