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 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
53 changes: 26 additions & 27 deletions src/trait_manager/model/TraitFactory.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,34 @@
import { TraitManagerConfig } from '../config/config';
import { TraitProperties } from './Trait';
import { isString } from 'underscore';
import Trait, { TraitProperties } from './Trait';

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): Trait {
return isString(prop) ? this.buildFromString(prop) : new Trait(prop);
}

for (let i = 0; i < props.length; i++) {
const prop = props[i];
const obj: TraitProperties = {
name: prop,
type: 'text',
};
private buildFromString(name: string): 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);
}
}
41 changes: 20 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 @@ -33,30 +37,25 @@ export default class Traits extends Collection<Trait> {
this.target = 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;
}

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);
traits[i].target = this.target;
}
return super.add(traits, opt);
}
const trait = models instanceof Trait ? models : this.tf.build(models as any);
trait.target = this.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