Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
146 changes: 99 additions & 47 deletions core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1280,9 +1280,9 @@ Montage.defineProperty(Montage.prototype, "version", {
* This is an evolution to progressively remove the reliance on the additional
* serializable property set on JS PropertyDescritpors, and instead relay on setting in ObjectDescriptors
* property descriptors. The range of value is unusual as it is a blend of string and boolean....
*
*
* Posible values: "reference" | "value" | "auto" | false,
*
*
* @type {string | boolean} .
*/

Expand All @@ -1303,7 +1303,7 @@ Montage.defineProperty(Montage.prototype, "_buildSerializablePropertyNames", {

let _serializablePropertyNames,
legacySerializablePropertyNames = Montage.getSerializablePropertyNames(this);

Montage.defineProperty(Object.getPrototypeOf(this), "_serializablePropertyNames", {
value: (_serializablePropertyNames = this.objectDescriptor
? this.objectDescriptor.serializablePropertyDescriptors.map((aPropertyDescriptor) => {
Expand Down Expand Up @@ -1356,60 +1356,112 @@ Montage.defineProperty(Montage, "equals", {
});

/**
* This method calls the method named with the identifier prefix if it exists.
* Example: If the name parameter is "shouldDoSomething" and the caller's identifier is "bob", then
* this method will try and call "bobShouldDoSomething"
* Calls the delegate method with the specified name if it exists on the delegate object.
* Uses caching to avoid repeated method lookups since delegate methods are unlikely to be removed dynamically.
*
* TODO: Cache!!!! We're unlikely to remove a delegate method dynamically, so we should avoid checking all
* that and just cache the function found, using a weak map, so don't retain delegates.
* The method first attempts to find a method with the pattern `{identifier}{Name}` on the delegate,
* where the first letter of the name parameter is capitalized. If not found, it falls back to
* looking for a method with the exact name.
*
* @function Montage#callDelegateMethod
* @param {string} name
*/
* @param {string} name - The name of the delegate method to call
* @param {...*} args - Additional arguments to pass to the delegate method
* @returns {*} The return value of the delegate method, or undefined if no method was found or no delegate exists
*
* @example
* // If this.identifier is "bob" and name is "shouldDoSomething"
* // This will try to call "bobShouldDoSomething" on the delegate
* const result = this.callDelegateMethod("shouldDoSomething", arg1, arg2);
*/
Montage.defineProperty(Montage.prototype, "callDelegateMethod", {
value: function (name) {
var delegate = this.delegate, delegateFunction;
const delegateFunction = this.getDelegateMethod(name);

if (delegate) {
if (this.delegate && delegateFunction) {
const [, ...rest] = arguments;
return delegateFunction.call(this.delegate, ...rest);
}
}
});

var delegateFunctionName = this.identifier;
delegateFunctionName += name.toCapitalized();
/**
* Checks whether the delegate object has a method that responds to the specified name.
* This method uses the same lookup logic as callDelegateMethod and getDelegateMethod.
*
* @function Montage#respondsToDelegateMethod
* @param {string} name - The name of the delegate method to check for
* @returns {boolean} True if the delegate has a method that responds to the given name, false otherwise
*
* @example
* // Check if delegate can handle "shouldDoSomething"
* if (this.respondsToDelegateMethod("shouldDoSomething")) {
* this.callDelegateMethod("shouldDoSomething", data);
* }
*/
Montage.defineProperty(Montage.prototype, "respondsToDelegateMethod", {
value: function (name) {
return typeof this.getDelegateMethod(name) === FUNCTION;
}
});

if (
typeof this.identifier === "string" &&
typeof delegate[delegateFunctionName] === FUNCTION
) {
delegateFunction = delegate[delegateFunctionName];
} else if (typeof delegate[name] === FUNCTION) {
delegateFunction = delegate[name];
}
// WeakMap to cache delegate methods - won't retain delegates when they're garbage collected
const delegateMethodCache = new WeakMap();

if (delegateFunction) {
//Using modern JS:
// Destructure the array to skip the first element
const [, ...rest] = arguments;
return delegateFunction.call(delegate, ...rest);

// if(arguments.length === 2) {
// return delegateFunction.call(delegate,arguments[1]);
// }
// else if(arguments.length === 3) {
// return delegateFunction.call(delegate,arguments[1],arguments[2]);
// }
// else if(arguments.length === 4) {
// return delegateFunction.call(delegate,arguments[1],arguments[2],arguments[3]);
// }
// else if(arguments.length === 5) {
// return delegateFunction.call(delegate,arguments[1],arguments[2],arguments[3],arguments[4]);
// }
// else {
// // remove first argument
// ARRAY_PROTOTYPE.shift.call(arguments);
// return delegateFunction.apply(delegate, arguments);
// }
}
/**
* Retrieves a delegate method by name, using caching for performance optimization.
*
* The method searches for delegate methods in the following order:
* 1. First tries `{identifier}{Name}` where Name is the capitalized version of the name parameter
* 2. Falls back to the exact method name if the prefixed version doesn't exist
*
* Results are cached using a WeakMap to avoid repeated lookups while ensuring
* delegates can still be garbage collected when no longer referenced.
*
* @function Montage#getDelegateMethod
* @param {string} name - The name of the delegate method to retrieve
* @returns {Function|undefined} The delegate method function if found, undefined otherwise
*
* @example
* // If this.identifier is "list" and name is "shouldSelectItem"
* // This will look for "listShouldSelectItem" first, then "shouldSelectItem"
* const method = this.getDelegateMethod("shouldSelectItem");
* if (method) {
* method.call(this.delegate, item);
* }
*/
Montage.defineProperty(Montage.prototype, "getDelegateMethod", {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for factoring that out, however mod doesn't use the get... naming pattern, and delegateMethod() isn't a good name for what this does as it ignores the argument passed in.

delegateMethodNamed(name) is misleading since if the receiver has an identifier property, it will change the name of the method it will try find on the delegate.

something like

delegateMethodWithName(name) would be better, it acknowledges the argument, but it's a bit fuzzy about what it does with it

delegateMethodFromName(name) acknowledges the argument, and also hint that it is a starting point.

delegateMethodDerivedFromName(name) is longer but more explicit. At least it prompts one to ask: How is it derived? What's the logic, wich is explained in the method description

value: function (name) {
if (!this.delegate) return;

const delegate = this.delegate;
let delegateCache = delegateMethodCache.get(delegate);

if (!delegateCache) {
delegateCache = new Map();
delegateMethodCache.set(delegate, delegateCache);
}
}

// Check if we already have the function cached
if (delegateCache.has(name)) return delegateCache.get(name);

let delegateFunctionName = this.identifier;
let delegateFunction;

delegateFunctionName += name.toCapitalized();

if (typeof this.identifier === "string" && typeof delegate[delegateFunctionName] === FUNCTION) {
delegateFunction = delegate[delegateFunctionName];
} else if (typeof delegate[name] === FUNCTION) {
delegateFunction = delegate[name];
}

// Cache the delegate function if it exists
if (delegateFunction) {
delegateCache.set(name, delegateFunction);
}

return delegateFunction;
},
});

// Property Changes
Expand Down
151 changes: 78 additions & 73 deletions ui/slot.mod/slot.js
Original file line number Diff line number Diff line change
@@ -1,101 +1,106 @@
/**
@module "mod/ui/slot.mod"
@requires mod/ui/component
*/
var Component = require("../component").Component;
* @module "mod/ui/slot.mod"
* @requires mod/ui/component
*/
const Component = require("../component").Component;
const Montage = require("core/core").Montage;

/**
* @class Slot
* @classdesc A structural component that serves as a place-holder for some
* other component.
* @extends Component
*/
exports.Slot = Component.specialize( /** @lends Slot.prototype # */ {
exports.Slot = class Slot extends Component {
static {
Montage.defineProperties(this.prototype, {
/**
* An optional helper object. The slot consults
* `delegate.slotElementForComponent(component):Element` if available for
* the element it should use when placing a particular component on the
* document. The slot informs `delegate.slotDidSwitchContent(slot,
* newContent, newComponent, oldContent, oldComponent)` if the content has
* finished changing. The component arguments are the `component`
* properties of the corresponding content, or fall back to `null`.
* @type {?Object}
* @default null
*/
delegate: { value: null },

hasTemplate: {
enumerable: false,
value: false
},
_content: { value: null },

/**
* An optional helper object. The slot consults
* `delegate.slotElementForComponent(component):Element` if available for
* the element it should use when placing a particular component on the
* document. The slot informs `delegate.slotDidSwitchContent(slot,
* newContent, newComponent, oldContent, oldComponent)` if the content has
* finished changing. The component arguments are the `component`
* properties of the corresponding content, or fall back to `null`.
* @type {?Object}
* @default null
*/
delegate: {
value: null
},

_content: {
value: null
},

enterDocument:{
value:function (firstTime) {
if (firstTime) {
this.element.classList.add("slot-mod");
}
}
},
hasTemplate: {
enumerable: false,
value: false,
},
});
}

get hasTemplate() {
return false;
}

/**
* The component that resides in this slot and in its place in the
* template.
* @type {Component}
* @default null
*/
content: {
get: function () {
return this._content;
},
set: function (value) {
var element,
content;

if (value && typeof value.needsDraw !== "undefined") {
content = this._content;

// If the incoming content was a component; make sure it has an element before we say it needs to draw
if (!value.element) {
element = document.createElement("div");

if (this.delegate && typeof this.delegate.slotElementForComponent === "function") {
element = this.delegate.slotElementForComponent(this, value, element);
}
value.element = element;
} else {
element = value.element;
}
*/
get content() {
return this._content;
}

set content(value) {
let element;

if (value && typeof value.needsDraw !== "undefined") {
// If the incoming content was a component;
// make sure it has an element before we say it needs to draw
if (!value.element) {
element = document.createElement("div");

// The child component will need to draw; this may trigger a draw for the slot itself
this.domContent = element;
value.needsDraw = true;
if (this.respondsToDelegateMethod("slotElementForComponent")) {
element = this.callDelegateMethod("slotElementForComponent", this, value, element);
}

value.element = element;
} else {
this.domContent = value;
element = value.element;
}

this._content = value;
this.needsDraw = true;
// The child component will need to draw;
// this may trigger a draw for the slot itself
this.domContent = element;
value.needsDraw = true;
} else {
this.domContent = value;
}
},

this._content = value;
this.needsDraw = true;
}

enterDocument(firstTime) {
if (firstTime) {
this.element.classList.add("slot-mod");
}

this.addEventListener("firstDraw", this, false);
}

exitDocument() {
this.removeEventListener("firstDraw", this, false);
}

handleFirstDraw() {
this.callDelegateMethod("slotContentDidFirstDraw", this);
Copy link
Contributor

Choose a reason for hiding this comment

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

That should technically be called "slotDidFirstDraw", no? We're listening for "firstDraw" on the slot, not on this.content. Should we listen to

    this.content.addEventListener("firstDraw", this, false);

instead?

}

/**
* Informs the `delegate` that `slotDidSwitchContent(slot)`
* @function
*/
contentDidChange: {
value: function () {
if (this.delegate && typeof this.delegate.slotDidSwitchContent === "function") {
this.delegate.slotDidSwitchContent(this);
}
}
contentDidChange() {
this.callDelegateMethod("slotDidSwitchContent", this);
Copy link
Contributor

Choose a reason for hiding this comment

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

In that kind of scenario, the content that went out and the one that came in should be passed to the delegate as arguments, like:

aDelegate.slotDidSwitchContent(this, oldContent, currentContent);

Also, that kind of delegate methods are often symmetric, with something like:

let delegateIncomingContent = aDelegate.slotWillSwitchContent(this, this.content, incomingContent);

Which gives the delegate the ability to override what's about to happen.

}

});
};