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

Handle missing mixins and applying mixin attributes to mappings of primitives #5483

Merged
merged 1 commit into from
Mar 14, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/core/a-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ var MULTIPLE_COMPONENT_DELIMITER = '__';
/**
* @member {object} componentCache - Cache of pre-parsed values. An object where the keys
* are component names and the values are already parsed by the component.
* @member {object} rawAttributeCache - Cache of the raw attribute values.
*/
class AMixin extends ANode {
constructor () {
super();
this.componentCache = {};
this.rawAttributeCache = {};
this.isMixin = true;
}

Expand Down Expand Up @@ -60,10 +62,11 @@ class AMixin extends ANode {
// Get component data.
componentName = utils.split(attr, MULTIPLE_COMPONENT_DELIMITER)[0];
component = components[componentName];
if (!component) { return; }
if (value === undefined) {
value = window.HTMLElement.prototype.getAttribute.call(this, attr);
}
this.rawAttributeCache[attr] = value;
if (!component) { return; }
this.componentCache[attr] = component.parseAttrValueForCache(value);
}

Expand Down
19 changes: 14 additions & 5 deletions src/core/a-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class ANode extends HTMLElement {
this.computedMixinStr = '';
this.mixinEls.length = 0;
for (i = 0; i < newMixinIds.length; i++) {
this.registerMixin(document.getElementById(newMixinIds[i]));
this.registerMixin(newMixinIds[i]);
}

// Update DOM. Keep track of `computedMixinStr` to not recurse back here after
Expand All @@ -240,21 +240,25 @@ class ANode extends HTMLElement {
/**
* From mixin ID, add mixin element to `mixinEls`.
*
* @param {Element} mixinEl
* @param {string} mixinId - ID of the mixin to register.
*/
registerMixin (mixinEl) {
registerMixin (mixinId) {
var compositedMixinIds;
var i;
var mixin;
var mixinEl = document.getElementById(mixinId);

if (!mixinEl) { return; }
if (!mixinEl) {
warn('No mixin was found with id `%s`', mixinId);
return;
}

// Register composited mixins (if mixin has mixins).
mixin = mixinEl.getAttribute('mixin');
if (mixin) {
compositedMixinIds = utils.split(mixin.trim(), /\s+/);
for (i = 0; i < compositedMixinIds.length; i++) {
this.registerMixin(document.getElementById(compositedMixinIds[i]));
this.registerMixin(compositedMixinIds[i]);
}
}

Expand All @@ -268,6 +272,11 @@ class ANode extends HTMLElement {
window.HTMLElement.prototype.setAttribute.call(this, attr, newValue);
}

/**
* Removes the mixin element from `mixinEls`.
*
* @param {string} mixinId - ID of the mixin to remove.
*/
unregisterMixin (mixinId) {
var i;
var mixinEls = this.mixinEls;
Expand Down
50 changes: 36 additions & 14 deletions src/extras/primitives/primitives.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ module.exports.registerPrimitive = function registerPrimitive (name, definition)
var i;
var mapping;
var mixins;
var path;
var self = this;

// Gather component data from default components.
Expand All @@ -79,12 +78,25 @@ module.exports.registerPrimitive = function registerPrimitive (name, definition)
// Factor in mixins to overwrite default components.
mixins = this.getAttribute('mixin');
if (mixins) {
mixins = mixins.trim().split(' ');
mixins = utils.split(mixins.trim(), /\s+/);
mixins.forEach(function applyMixin (mixinId) {
var mixinComponents = self.sceneEl.querySelector('#' + mixinId).componentCache;
Object.keys(mixinComponents).forEach(function setComponent (name) {
data[name] = extend(data[name], mixinComponents[name]);
});
var mixinEl = document.getElementById(mixinId);
if (!mixinEl) { return; }
var rawAttributeCache = mixinEl.rawAttributeCache;
var mixinComponents = mixinEl.componentCache;
for (var name in rawAttributeCache) {
// Check if the attribute matches a mapping.
mapping = self.mappings[name];
if (mapping) {
applyMapping(mapping, rawAttributeCache[name], data);
return;
}

// Check if the attribute belongs to a component.
if (name in mixinComponents) {
data[name] = extend(data[name], mixinComponents[name]);
}
}
});
}

Expand All @@ -93,14 +105,7 @@ module.exports.registerPrimitive = function registerPrimitive (name, definition)
attr = this.attributes[i];
mapping = this.mappings[attr.name];
if (mapping) {
path = utils.entity.getComponentPropertyPath(mapping);
if (path.constructor === Array) {
data[path[0]] = data[path[0]] || {};
data[path[0]][path[1]] = attr.value.trim();
} else {
data[path] = attr.value.trim();
}
continue;
applyMapping(mapping, attr.value, data);
}
}

Expand Down Expand Up @@ -169,6 +174,23 @@ module.exports.registerPrimitive = function registerPrimitive (name, definition)
return primitiveClass;
};

/**
* Sets the relevant property based on the mapping property path.
*
* @param {string} mapping - The mapped property path.
* @param {string} attrValue - The (raw) attribute value.
* @param {object} data - The data object to apply the mapping to.
*/
function applyMapping (mapping, attrValue, data) {
var path = utils.entity.getComponentPropertyPath(mapping);
if (path.constructor === Array) {
data[path[0]] = data[path[0]] || {};
data[path[0]][path[1]] = attrValue.trim();
} else {
data[path] = attrValue.trim();
}
}

/**
* Add component mappings using schema.
*/
Expand Down
17 changes: 17 additions & 0 deletions tests/extras/primitives/primitives.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,23 @@ suite('registerPrimitive (using innerHTML)', function () {
});
});

test('applies mappings to mixin attributes', function (done) {
AFRAME.registerComponent('test', {
schema: {default: 'foo'}
});
primitiveFactory({
defaultComponents: {
material: {color: 'blue'}
},
mappings: {foo: 'material.color'}
}, 'mixin="bar"', function postCreation (el) {
assert.equal(el.getAttribute('material').color, 'purple');
done();
}, function preCreation (sceneEl) {
helpers.mixinFactory('bar', {foo: 'purple'}, sceneEl);
});
});

test('handles mapping to a single-property component', function (done) {
primitiveFactory({
mappings: {
Expand Down
Loading