-
Notifications
You must be signed in to change notification settings - Fork 2
Slot improvements #52
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
base: main
Are you sure you want to change the base?
Changes from all commits
f4a37bd
47aa7a5
84e32d7
eec83c9
db194fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| } | ||
|
|
||
| }); | ||
| }; | ||
There was a problem hiding this comment.
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