FYI, we're legally obliged to have you submit any potential patches through JIRA, because the act of checking the license grant checkbox is important from a legal perspective.
-T On Thu, Sep 9, 2010 at 4:58 PM, Roger L. Whitcomb <[email protected]> wrote: > 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. >>>> >>>> >>>> >>>> >>>> >>> >>> > >
