You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Raymond Auge <ra...@liferay.com> on 2008/04/09 05:33:13 UTC

manual gadget.id

Hey All,

I'm seeing a bug in our container with respect to positioning of a
gadget on the page.

Currently, the gadget.id is simply a numerically incremented value that
is assigned based on the ordering of the gadget on the page. Moving the
gadget to a different position causes the prefs to be lost if the
ordering of rendering changes. Since we have drag and drop positioning
this is a little annoying.

I'm going to be overriding the container's default addGadget function
with an implementation which allows setting the gadget.id distinctly
rather than relying on an incremented value. Looking through the code it
doesn't appear that it really matters if the id is numerical or a
string.

So, I'm tempted to do this


gadgets.Container.prototype.addGadget = function(gadget) {
  if (!gadget.id) {
    gadget.id = this.getNextGadgetInstanceId();
  }
  gadget.setUserPrefs(this.userPrefStore.getPrefs(gadget));
  this.gadgets_[this.getGadgetKey_(gadget.id)] = gadget;
};


This way I can set the gadget.id prior to passing it to
gadgets.container.addGadget(gadget).

Any reason why this shouldn't be done?


Raymond Augé
Software Engineer
Liferay, Inc.
Enterprise. Open Source. For Life.

Re: manual gadget.id

Posted by Raymond Auge <ra...@liferay.com>.
Ok, ignore this.. I found a much easier solution by just adding an
expando to the gadget object. Now my prefs are stick with drag and drop
positioning.


Ray


On Wed, 2008-04-09 at 08:42 -0400, Raymond Auge wrote:

> Ok so,
> 
> We start with this:
> 
> 
> /**
>  * Sets chrome ids, whose indexes are gadget instance ids (starting from 0).
>  * @param {Array} gadgetIdToChromeIdMap Gadget id to chrome id map
>  */
> gadgets.StaticLayoutManager.prototype.setGadgetChromeIds =
>     function(gadgetChromeIds) {
>   this.gadgetChromeIds_ = gadgetChromeIds;
> };
> 
> 
> which only suites the case where all the gadgetChromeIds are available
> all at the same time. This doesn't work for us. I add this method, so I
> could add the chromes without having to do it all together:
> 
> 
> gadgets.StaticLayoutManager.prototype.addGadgetChromeId = function(gadgetChromeId) {
> 	if (this.gadgetChromeIds_ && gadgets.util.isArray(this.gadgetChromeIds_)) {
>   		this.gadgetChromeIds_.push(gadgetChromeId);
> 	}
> 	else {
>   		this.gadgetChromeIds_ = [gadgetChromeId];
> 	}
> };
> 
> 
> This worked exactly like we want and didn't break anything.
> 
> Later, due to the numerical gadget.id issue, I changed it to
> 
> 
> gadgets.StaticLayoutManager.prototype.addGadgetChromeId = function(gadgetId, gadgetChromeId) {
> 	if (!this.gadgetChromeIds_) {
> 	  	this.gadgetChromeIds_ = [];
> 	}
> 	
> 	this.gadgetChromeIds_[gadgetId] = gadgetChromeId;
> };
> 
> 
> So, my code for loading a given gadget is:
> 
> 
> <script type="text/javascript">
> jQuery(
> 	function() {
> 		var gadget = gadgets.container.createGadget({<%= gadgetParams %>});
> 
> 		gadget.id = '<portlet:namespace />';
> 
> 		gadget.secureToken = ['<%= ownerId %>', themeDisplay.getUserId(), "<portlet:namespace />gadget", "liferay"].join(":");
> 
> 		gadget.setServerBase('<%= renderRequest.getContextPath() %>/');
> 
> 		gadgets.container.addGadget(gadget);
> 		gadgets.container.layoutManager.addGadgetChromeId(gadget.id, '<portlet:namespace />gadget-chrome');
> 	}
> );
> </script>
> 
> 
> Then I had a look through HttpGadgetContext which has an Integer type
> moduleId
> 
>   public HttpGadgetContext(HttpServletRequest request) {
>     ...
>     moduleId = getModuleId(request);
>     ...
>   }
> 
> but everywhere the moduleId is used it is essentially converted back to
> a string and the only reference I find of the integer aspect being used
> is line 120 of UrlGenerator
> 
>       ...
>       if (context.getModuleId() != 0) {
>       ...
> 
> which could just as easily be a string check...
> 
> So, can we change the context moduleId to a string rather than an int?
> 
> Note that I haven't looked at the PHP implementation, so I don't know
> how it might be affected by the change (though unless the PHP impl is
> using strict casting, I doubt it would require a change at all).
> 
> Ray
> 
> On Tue, 2008-04-08 at 23:33 -0400, Raymond Auge wrote:
> 
> > Hey All,
> > 
> > I'm seeing a bug in our container with respect to positioning of a
> > gadget on the page.
> > 
> > Currently, the gadget.id is simply a numerically incremented value that
> > is assigned based on the ordering of the gadget on the page. Moving the
> > gadget to a different position causes the prefs to be lost if the
> > ordering of rendering changes. Since we have drag and drop positioning
> > this is a little annoying.
> > 
> > I'm going to be overriding the container's default addGadget function
> > with an implementation which allows setting the gadget.id distinctly
> > rather than relying on an incremented value. Looking through the code it
> > doesn't appear that it really matters if the id is numerical or a
> > string.
> > 
> > So, I'm tempted to do this
> > 
> > 
> > gadgets.Container.prototype.addGadget = function(gadget) {
> >   if (!gadget.id) {
> >     gadget.id = this.getNextGadgetInstanceId();
> >   }
> >   gadget.setUserPrefs(this.userPrefStore.getPrefs(gadget));
> >   this.gadgets_[this.getGadgetKey_(gadget.id)] = gadget;
> > };
> > 
> > 
> > This way I can set the gadget.id prior to passing it to
> > gadgets.container.addGadget(gadget).
> > 
> > Any reason why this shouldn't be done?
> > 
> > 
> > Raymond Augé
> > Software Engineer
> > Liferay, Inc.
> > Enterprise. Open Source. For Life.
> 
> Raymond Augé
> Software Engineer
> Liferay, Inc.
> Enterprise. Open Source. For Life.

Raymond Augé
Software Engineer
Liferay, Inc.
Enterprise. Open Source. For Life.

Re: manual gadget.id

Posted by Raymond Auge <ra...@doublebite.com>.
Ok so,

We start with this:


/**
 * Sets chrome ids, whose indexes are gadget instance ids (starting from 0).
 * @param {Array} gadgetIdToChromeIdMap Gadget id to chrome id map
 */
gadgets.StaticLayoutManager.prototype.setGadgetChromeIds =
    function(gadgetChromeIds) {
  this.gadgetChromeIds_ = gadgetChromeIds;
};


which only suites the case where all the gadgetChromeIds are available
all at the same time. This doesn't work for us. I add this method, so I
could add the chromes without having to do it all together:


gadgets.StaticLayoutManager.prototype.addGadgetChromeId = function(gadgetChromeId) {
	if (this.gadgetChromeIds_ && gadgets.util.isArray(this.gadgetChromeIds_)) {
  		this.gadgetChromeIds_.push(gadgetChromeId);
	}
	else {
  		this.gadgetChromeIds_ = [gadgetChromeId];
	}
};


This worked exactly like we want and didn't break anything.

Later, due to the numerical gadget.id issue, I changed it to


gadgets.StaticLayoutManager.prototype.addGadgetChromeId = function(gadgetId, gadgetChromeId) {
	if (!this.gadgetChromeIds_) {
	  	this.gadgetChromeIds_ = [];
	}
	
	this.gadgetChromeIds_[gadgetId] = gadgetChromeId;
};


So, my code for loading a given gadget is:


<script type="text/javascript">
jQuery(
	function() {
		var gadget = gadgets.container.createGadget({<%= gadgetParams %>});

		gadget.id = '<portlet:namespace />';

		gadget.secureToken = ['<%= ownerId %>', themeDisplay.getUserId(), "<portlet:namespace />gadget", "liferay"].join(":");

		gadget.setServerBase('<%= renderRequest.getContextPath() %>/');

		gadgets.container.addGadget(gadget);
		gadgets.container.layoutManager.addGadgetChromeId(gadget.id, '<portlet:namespace />gadget-chrome');
	}
);
</script>


Then I had a look through HttpGadgetContext which has an Integer type
moduleId

  public HttpGadgetContext(HttpServletRequest request) {
    ...
    moduleId = getModuleId(request);
    ...
  }

but everywhere the moduleId is used it is essentially converted back to
a string and the only reference I find of the integer aspect being used
is line 120 of UrlGenerator

      ...
      if (context.getModuleId() != 0) {
      ...

which could just as easily be a string check...

So, can we change the context moduleId to a string rather than an int?

Note that I haven't looked at the PHP implementation, so I don't know
how it might be affected by the change (though unless the PHP impl is
using strict casting, I doubt it would require a change at all).

Ray

On Tue, 2008-04-08 at 23:33 -0400, Raymond Auge wrote:

> Hey All,
> 
> I'm seeing a bug in our container with respect to positioning of a
> gadget on the page.
> 
> Currently, the gadget.id is simply a numerically incremented value that
> is assigned based on the ordering of the gadget on the page. Moving the
> gadget to a different position causes the prefs to be lost if the
> ordering of rendering changes. Since we have drag and drop positioning
> this is a little annoying.
> 
> I'm going to be overriding the container's default addGadget function
> with an implementation which allows setting the gadget.id distinctly
> rather than relying on an incremented value. Looking through the code it
> doesn't appear that it really matters if the id is numerical or a
> string.
> 
> So, I'm tempted to do this
> 
> 
> gadgets.Container.prototype.addGadget = function(gadget) {
>   if (!gadget.id) {
>     gadget.id = this.getNextGadgetInstanceId();
>   }
>   gadget.setUserPrefs(this.userPrefStore.getPrefs(gadget));
>   this.gadgets_[this.getGadgetKey_(gadget.id)] = gadget;
> };
> 
> 
> This way I can set the gadget.id prior to passing it to
> gadgets.container.addGadget(gadget).
> 
> Any reason why this shouldn't be done?
> 
> 
> Raymond Augé
> Software Engineer
> Liferay, Inc.
> Enterprise. Open Source. For Life.

Raymond Augé
Software Engineer
Liferay, Inc.
Enterprise. Open Source. For Life.