On 3/1/08 2:25 PM, "Kevin Brown" <[EMAIL PROTECTED]> wrote:
[....]
>> +  return {
>> +    /**
>> +     * Override the default properties with a new set of properties.
>> +     *
>> +     * @param {Object} properties The mapping of property names to values
>> +     */
>> +    init : function(properties) {
>> +      skinProperties = properties;
>> +    },
> 
> 
> This seems redundant -- you already have it wired to the normal config
> registration.

Correct, the config registration handles the default skin properties
for an entire site.  Hi5 allows an individual to choose the skin for
their profile page, which is passed into this init() method.
 
Do you want to change the naming of this function?   Or should I add
more documentation to describe it's usage?
 
Additionally we may want to allow overriding only a subset of
properties.  This is really a shindig implementation detail, we just
need to decide how the container + syndicator + skins work together.

>> +    getProperty : function(propertyKey) {
>> +
>> +      var property = skinProperties[propertyKey];
>> +      if (property) {
>> +        return property;
>> +      }
>> +      return "";
> 
> 
> You can shorten this to return skinProperties[propertyKey] || "";
> (coincidentally,  it looks like  the skins API spec needs to be amended to
> be specific about what  is supposed to be returned for invalid properties).

Okay, fix applied.

>> +  /**
>> +   * The repeat characteristics for the background image
>> +   * @member gadgets.skins.Property
>> +   */
>> +  BG_REPEAT : 'BG_REPEAT',
>> +
>> +  /**
>>    * The color that anchor tags should use.
>>    * @member gadgets.skins.Property
>>    */
>>   ANCHOR_COLOR : 'ANCHOR_COLOR'
>> +
>>  };
> 
> We should probalby use gadgets.util.makeEnum() for this.

Will that require us to tweak the spec?  If not I'll look at fixing
This.

Reply via email to