Second try: the attachments didn't go through I guess... Below is a diff of these 4 classes reimplemented as subclasses of "Content.java" (source also below, which is really just a renamed clone of what was in TreeNode.java). This is from 1.5.x branch. I rebuilt with these changes and tried my app again and all seems to be well.
Content.java: ------------- /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. * The ASF licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except in * compliance with the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package org.apache.pivot.wtk.content; import java.net.URL; import org.apache.pivot.util.ThreadUtilities; import org.apache.pivot.util.concurrent.TaskExecutionException; import org.apache.pivot.wtk.ApplicationContext; import org.apache.pivot.wtk.media.Image; /** * Default content node implementation. */ public class Content { protected Image icon = null; protected String text = null; protected Object userData = null; public Content() { this(null, null); } public Content(Image icon) { this(icon, null); } public Content(String text) { this(null, text); } public Content(Image icon, String text) { this.icon = icon; this.text = text; } public Image getIcon() { return icon; } public void setIcon(Image icon) { this.icon = icon; } /** * Sets the icon by URL. * <p> * <b>Note</b>: Using this signature will cause an entry to be added in the * application context's {...@linkplain ApplicationContext#getResourceCache() * resource cache} if one does not already exist. * * @param iconURL * The location of the icon to set. */ public void setIcon(URL iconURL) { if (iconURL == null) { throw new IllegalArgumentException("iconURL is null."); } Image icon = (Image)ApplicationContext.getResourceCache().get(iconURL); if (icon == null) { try { icon = Image.load(iconURL); } catch (TaskExecutionException exception) { throw new IllegalArgumentException(exception); } ApplicationContext.getResourceCache().put(iconURL, icon); } setIcon(icon); } /** * Sets the icon by {...@linkplain ClassLoader#getResource(String) * resource name}. * <p> * <b>Note</b>: Using this signature will cause an entry to be added in the * application context's {...@linkplain ApplicationContext#getResourceCache() * resource cache} if one does not already exist. * * @param iconName * The resource name of the icon to set. */ public void setIcon(String iconName) { if (iconName == null) { throw new IllegalArgumentException("iconName is null."); } ClassLoader classLoader = ThreadUtilities.getClassLoader(); setIcon(classLoader.getResource(iconName)); } public String getText() { return text; } public void setText(String text) { this.text = text; } public Object getUserData() { return userData; } public void setUserData(Object userData) { this.userData = userData; } } Diffs: ------ Index: wtk/src/org/apache/pivot/wtk/content/TreeNode.java =================================================================== --- wtk/src/org/apache/pivot/wtk/content/TreeNode.java (revision 993445) +++ wtk/src/org/apache/pivot/wtk/content/TreeNode.java (working copy) @@ -16,22 +16,13 @@ */ package org.apache.pivot.wtk.content; -import java.net.URL; - -import org.apache.pivot.util.ThreadUtilities; -import org.apache.pivot.util.concurrent.TaskExecutionException; -import org.apache.pivot.wtk.ApplicationContext; import org.apache.pivot.wtk.media.Image; /** * Default tree node implementation. */ -public class TreeNode { - private Image icon = null; - private String text = null; - private Object userData = null; - +public class TreeNode extends Content { public TreeNode() { this(null, null); } @@ -45,81 +36,7 @@ } public TreeNode(Image icon, String text) { - this.icon = icon; - this.text = text; + super(icon, text); } - public Image getIcon() { - return icon; - } - - public void setIcon(Image icon) { - this.icon = icon; - } - - /** - * Sets the tree node's icon by URL. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconURL - * The location of the icon to set. - */ - public void setIcon(URL iconURL) { - if (iconURL == null) { - throw new IllegalArgumentException("iconURL is null."); - } - - Image icon = (Image)ApplicationContext.getResourceCache().get(iconURL); - - if (icon == null) { - try { - icon = Image.load(iconURL); - } catch (TaskExecutionException exception) { - throw new IllegalArgumentException(exception); - } - - ApplicationContext.getResourceCache().put(iconURL, icon); - } - - setIcon(icon); - } - - /** - * Sets the tree node's icon by {...@linkplain ClassLoader#getResource(String) - * resource name}. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconName - * The resource name of the icon to set. - */ - public void setIcon(String iconName) { - if (iconName == null) { - throw new IllegalArgumentException("iconName is null."); - } - - ClassLoader classLoader = ThreadUtilities.getClassLoader(); - setIcon(classLoader.getResource(iconName)); - } - - public String getText() { - return text; - } - - public void setText(String text) { - this.text = text; - } - - public Object getUserData() { - return userData; - } - - public void setUserData(Object userData) { - this.userData = userData; - } } Index: wtk/src/org/apache/pivot/wtk/content/TableViewHeaderData.java =================================================================== --- wtk/src/org/apache/pivot/wtk/content/TableViewHeaderData.java (revision 993445) +++ wtk/src/org/apache/pivot/wtk/content/TableViewHeaderData.java (working copy) @@ -16,20 +16,13 @@ */ package org.apache.pivot.wtk.content; -import java.net.URL; - -import org.apache.pivot.util.ThreadUtilities; -import org.apache.pivot.util.concurrent.TaskExecutionException; -import org.apache.pivot.wtk.ApplicationContext; import org.apache.pivot.wtk.media.Image; /** * Default table header data implementation. */ -public class TableViewHeaderData { - private Image icon = null; - private String text = null; +public class TableViewHeaderData extends Content { public TableViewHeaderData() { this(null, null); @@ -44,73 +37,7 @@ } public TableViewHeaderData(Image icon, String text) { - this.icon = icon; - this.text = text; + super(icon, text); } - public Image getIcon() { - return icon; - } - - public void setIcon(Image icon) { - this.icon = icon; - } - - /** - * Sets the header data's icon by URL. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconURL - * The location of the icon to set. - */ - public void setIcon(URL iconURL) { - if (iconURL == null) { - throw new IllegalArgumentException("iconURL is null."); - } - - Image icon = (Image)ApplicationContext.getResourceCache().get(iconURL); - - if (icon == null) { - try { - icon = Image.load(iconURL); - } catch (TaskExecutionException exception) { - throw new IllegalArgumentException(exception); - } - - ApplicationContext.getResourceCache().put(iconURL, icon); - } - - setIcon(icon); - } - - /** - * Sets the header data's icon by {...@linkplain ClassLoader#getResource(String) - * resource name}. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconName - * The resource name of the icon to set. - */ - public void setIcon(String iconName) { - if (iconName == null) { - throw new IllegalArgumentException("iconName is null."); - } - - ClassLoader classLoader = ThreadUtilities.getClassLoader(); - setIcon(classLoader.getResource(iconName)); - } - - public String getText() { - return text; - } - - public void setText(String text) { - this.text = text; - } } Index: wtk/src/org/apache/pivot/wtk/content/ButtonData.java =================================================================== --- wtk/src/org/apache/pivot/wtk/content/ButtonData.java (revision 993445) +++ wtk/src/org/apache/pivot/wtk/content/ButtonData.java (working copy) @@ -16,20 +16,13 @@ */ package org.apache.pivot.wtk.content; -import java.net.URL; - -import org.apache.pivot.util.ThreadUtilities; -import org.apache.pivot.util.concurrent.TaskExecutionException; -import org.apache.pivot.wtk.ApplicationContext; import org.apache.pivot.wtk.media.Image; /** * Default button data implementation. */ -public class ButtonData { - private Image icon; - private String text; +public class ButtonData extends Content { public ButtonData() { this(null, null); @@ -44,73 +37,7 @@ } public ButtonData(Image icon, String text) { - this.icon = icon; - this.text = text; + super(icon, text); } - public Image getIcon() { - return icon; - } - - public void setIcon(Image icon) { - this.icon = icon; - } - - /** - * Sets the button data's icon by URL. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconURL - * The location of the icon to set. - */ - public void setIcon(URL iconURL) { - if (iconURL == null) { - throw new IllegalArgumentException("iconURL is null."); - } - - Image icon = (Image)ApplicationContext.getResourceCache().get(iconURL); - - if (icon == null) { - try { - icon = Image.load(iconURL); - } catch (TaskExecutionException exception) { - throw new IllegalArgumentException(exception); - } - - ApplicationContext.getResourceCache().put(iconURL, icon); - } - - setIcon(icon); - } - - /** - * Sets the button data's icon by {...@linkplain ClassLoader#getResource(String) - * resource name}. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconName - * The resource name of the icon to set. - */ - public void setIcon(String iconName) { - if (iconName == null) { - throw new IllegalArgumentException("iconName is null."); - } - - ClassLoader classLoader = ThreadUtilities.getClassLoader(); - setIcon(classLoader.getResource(iconName)); - } - - public String getText() { - return text; - } - - public void setText(String text) { - this.text = text; - } } Index: wtk/src/org/apache/pivot/wtk/content/ListItem.java =================================================================== --- wtk/src/org/apache/pivot/wtk/content/ListItem.java (revision 993445) +++ wtk/src/org/apache/pivot/wtk/content/ListItem.java (working copy) @@ -16,20 +16,13 @@ */ package org.apache.pivot.wtk.content; -import java.net.URL; - -import org.apache.pivot.util.ThreadUtilities; -import org.apache.pivot.util.concurrent.TaskExecutionException; -import org.apache.pivot.wtk.ApplicationContext; import org.apache.pivot.wtk.media.Image; /** * Default list item implementation. */ -public class ListItem { - private Image icon; - private String text; +public class ListItem extends Content { public ListItem() { this(null, null); @@ -44,73 +37,7 @@ } public ListItem(Image icon, String text) { - this.icon = icon; - this.text = text; + super(icon, text); } - public Image getIcon() { - return icon; - } - - public void setIcon(Image icon) { - this.icon = icon; - } - - /** - * Sets the list item's icon by URL. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconURL - * The location of the icon to set. - */ - public void setIcon(URL iconURL) { - if (iconURL == null) { - throw new IllegalArgumentException("iconURL is null."); - } - - Image icon = (Image)ApplicationContext.getResourceCache().get(iconURL); - - if (icon == null) { - try { - icon = Image.load(iconURL); - } catch (TaskExecutionException exception) { - throw new IllegalArgumentException(exception); - } - - ApplicationContext.getResourceCache().put(iconURL, icon); - } - - setIcon(icon); - } - - /** - * Sets the list item's icon by {...@linkplain ClassLoader#getResource(String) - * resource name}. - * <p> - * <b>Note</b>: Using this signature will cause an entry to be added in the - * application context's {...@linkplain ApplicationContext#getResourceCache() - * resource cache} if one does not already exist. - * - * @param iconName - * The resource name of the icon to set. - */ - public void setIcon(String iconName) { - if (iconName == null) { - throw new IllegalArgumentException("iconName is null."); - } - - ClassLoader classLoader = ThreadUtilities.getClassLoader(); - setIcon(classLoader.getResource(iconName)); - } - - public String getText() { - return text; - } - - public void setText(String text) { - this.text = text; - } } Roger Whitcomb | Architect, Engineering | [email protected] | Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | USA +1 650-587-5596 | fax: +1 650-587-5550 -----Original Message----- From: Roger L. Whitcomb [mailto:[email protected]] Sent: Thursday, September 09, 2010 12:14 PM To: [email protected] Cc: [email protected] Subject: RE: Best practice for implementing Actions Actually, if you made a base class Content with three fields: String text, Image icon and Object userData, then both ButtonData and TreeNode could inherit from Content with nothing else added to either.... Also ListItem and TableViewHeaderData. Roger Whitcomb | Architect, Engineering | [email protected] | Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | USA +1 650-587-5596 | fax: +1 650-587-5550 -----Original Message----- From: Greg Brown [mailto:[email protected]] Sent: Thursday, September 09, 2010 10:45 AM To: [email protected] Cc: [email protected] Subject: Re: Best practice for implementing Actions Quite possibly, though it isn't as critical for ButtonData, since you could easily create your own ButtonData subclass. This would have been more of a pain for the tree data, since you would have had to create two custom subclasses (one for TreeNode and another for TreeBranch). OTOH, it may not be a bad idea to add a user data property to all of the default content classes. On Sep 9, 2010, at 1:41 PM, Roger L. Whitcomb wrote: > Just wondering: you just added a "userData" member to TreeNode to help with context, would a "userData" member of ButtonData be appropriate/helpful here as well? > > Roger Whitcomb | Architect, Engineering | [email protected] | Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | USA +1 650-587-5596 | fax: +1 650-587-5550 > > -----Original Message----- > From: Roger L. Whitcomb [mailto:[email protected]] > Sent: Thursday, September 09, 2010 10:38 AM > To: [email protected]; [email protected] > Subject: RE: Best practice for implementing Actions > > How did you do the "dependency injection" within the actions? I'm looking at my code, and I'm thinking along these lines too. Having the invoking component (esp. the MenuItem) helps for some things, but, as you say, often you need other kinds of context within the application, regardless of the UI context. > > Roger Whitcomb | Architect, Engineering | [email protected] | Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | USA +1 650-587-5596 | fax: +1 650-587-5550 > > -----Original Message----- > From: Todd Volkert [mailto:[email protected]] > Sent: Thursday, September 09, 2010 10:33 AM > To: [email protected] > Cc: [email protected] > Subject: Re: Best practice for implementing Actions > > FWIW, I ran into this issue in one of my Pivot apps and took a > different approach altogether. Instead of looking at the context from > which the action was invoked, I looked at the state of the app in the > abstract sense. > > For instance, I might look at the selected path of a TreeView to see > which node they right-clicked on, or I might look at the selected item > of a list view cross-referenced with the selected items of a table > view. > > I my cases, I needed more than just how they were invoked. Very > specific to my case, I used dependency injection within the actions to > get the necessary state without hard-coupling it to the UI. > > This doesn't invalidate the thought that we might want to pass the > "invocation context" to the action's perform() method, but maybe it's > food for thought. > > Cheers, > -T > > On Thu, Sep 9, 2010 at 1:24 PM, Roger L. Whitcomb > <[email protected]> wrote: >> The Stock Tracker tutorial has it in 1.5.x also, I just left out the >> Tutorial code from my list. And yes, the Component is available in >> there as well. >> >> Let me think about how that works in what I have prototyped as well. I >> think that helps with the ugly case of having global variables that are >> referenced directly by class name. >> >> Roger Whitcomb | Architect, Engineering | [email protected] | >> Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | >> USA +1 650-587-5596 | fax: +1 650-587-5550 >> >> -----Original Message----- >> From: Greg Brown [mailto:[email protected]] >> Sent: Thursday, September 09, 2010 10:10 AM >> To: [email protected] >> Cc: [email protected] >> Subject: Re: Best practice for implementing Actions >> >> Yes, that is exactly what I did. There is also some code in the Stock >> Tracker tutorial that calls perform() (it may only be in the 2.0 >> branch), but a Component argument also works there. >> >> On Sep 9, 2010, at 1:07 PM, Roger L. Whitcomb wrote: >> >>> So, looking for where the Action.perform() method is called from: >>> >>> - In Window.java from a key press - the context would have to >>> be the Window component >>> >>> - In Button.java - obviously the Button component (Menu.Item >>> and MenuBar.Item are both derived from this) >>> >>> That's all the places I can find - I guess that covers all the cases? >>> So, Component it should be. >>> >>> >>> >>> Roger Whitcomb | Architect, Engineering | [email protected]| >>> Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 | >>> USA >>> >> <http://www.google.com/maps?f=q&hl=en&geocode=&q=500+Arguello+Street+%7C >>> >> +Suite+200+%7C+Redwood+City+%7C+CA+%7C+94063+%7C+USA+&sll=37.0625,-95.67 >>> 7068&sspn=50.557552,73.037109&ie=UTF8&t=h&z=16&iwloc=addr> | +1 >>> 650-587-5596 | fax: +1 650-587-5550 >>> >>> From: Greg Brown [mailto:[email protected]] >>> Sent: Thursday, September 09, 2010 9:55 AM >>> To: [email protected] >>> Cc: Pivot Dev >>> Subject: Re: Best practice for implementing Actions >>> >>> >>> >>> Ha. I think you have identified a valid issue with the Action >> interface: >>> the perform() method does not provide the implementor with any >>> information about the object that triggered the action. >>> >>> >>> >>> Now, this was originally by design - the thinking was that it should >> be >>> possible to execute an action regardless of the UI element that >> invoked >>> it. Since actions can potentially be triggered by multiple elements, >>> they should not generally have a dependency on a particular element. >>> However, that premise doesn't account for shared actions, or the >>> possibility that the developer may actually want to execute different >>> action behaviors based on the source value. >>> >>> >>> >>> In order to resolve this, I think we'll need to add an argument to the >>> perform() method that identifies the action's source. If we add it to >>> 1.5.2, it will be a breaking API change, which we generally try to >> avoid >>> in maintenance releases. However, I have already made one API change >> for >>> 1.5.2 (to the ResultList class, to resolve a serious performance >> issue), >>> so it is not out of the question. Since this is a major functional >>> limitation, it is probably worth doing. >>> >>> >>> >>> The only question in my mind is - what should the type of the source >>> argument be? It could be an Object, but I could also see an argument >> for >>> making it a Component (Action is defined in the org.apache.pivot.wtk >>> package, after all). >>> >>> >>> >>> Thoughts/comments? >>> >>> >>> >>> Greg >>> >>> >>> >>> On Sep 9, 2010, at 10:33 AM, Roger L. Whitcomb wrote: >>> >>> >>> >>> >>> >>> My application has a large tree in the left-hand side with context >> menus >>> (different) on every node of the tree. So, the context menu actions >> are >>> highly context-sensitive (the "Properties" item, obviously, would need >>> to know the exact selected object that the context menu was brought up >>> on in order to get the right Properties dialog). >>> >>> >>> >>> So, my question is: what is "best practice" as far as Pivot goes in >>> order to transmit this context into the Action.perform() method? The >>> "Expenses" tutorial seems to rely on global variables and using >>> "getSelectedRow" on that TableView object. So, is this the best way? >>> Is there some way I don't see to get the context directly in the >> Action >>> object? >>> >>> >>> >>> Thanks. >>> >>> >>> >>> Roger Whitcomb >>> >>> Architect, Engineering >>> >>> Ingres Corporation >>> >>> [email protected] >>> >>> >>> >>> PHONE +1 650.587.5596 >>> >>> FAX +1 650.587.5550 >>> >>> >>> >>> www.ingres.com <http://www.ingres.com/> >>> >>> >>> >>> This transmission is confidential and intended solely for the use of >> the >>> recipient named above. It may contain confidential, proprietary, or >>> legally privileged information. If you are not the intended recipient, >>> you are hereby notified that any unauthorized review, use, disclosure >> or >>> distribution is strictly prohibited. If you have received this >>> transmission in error, please contact the sender by reply e-mail and >>> delete the original transmission and all copies from your system. >>> >>> >>> >>> >>> >> >>
