Community
Participate
Working Groups
Created attachment 238033 [details] Proposed new Interface I require the ability to change the font (and maybe color) of entries within the Properties View. Ideally, this would be achieved by provider a LabelProvider to the PropertyDescriptor that optionally implements the IColorProvider and IFontProvider interfaces. I assume we'd need an IPropertySheetEntry2 interface (which extends IPropertySheetEntry) to provide methods to access this information. The PropertySheetEntry class need to implement the new interface, and support the additional methods to provide this information. The PropertySheetViewer need to implement the new interface, and the updateEntry method needs to usse this new information. e.g. private void updateEntry(IPropertySheetEntry entry, TreeItem item) { ... if (entry instanceof IPropertySheetEntry2) { IPropertySheetEntry2 entry2 = (IPropertySheetEntry2) entry; Color color = entry2.getForeground(); if (item.getForeground() != color) { item.setForeground(color); } color = entry2.getBackground(); if (item.getBackground()!= color) { item.setBackground(color); } Font font = entry2.getFont(); if (item.getFont() != font) { item.setFont(font); } } With these modification, existing entries deriving from PropertySheetEntry gain the new support, with no change evident for existing LabelProviders.
Changes submitted via gerrit.. https://git.eclipse.org/r/19366
Hi Kevin, I'm still not sure where this is going. Could you please attach a small sample plugin that shows how this is used? PW
Created attachment 238331 [details] Sample plugin The attached sample code makes use of the desired behaviour. bug423214sample.views.SampleView provide the interaction. run the plugin as an eclipse application open the Properties View open the Bug42312/Sample View click on an item in the sample view to have the properties show.
Created attachment 238332 [details] example of the changes in use
Thanks Kevin, We have contribution days next week, we can look at this. PW
(In reply to Kevin Milburn from comment #1) > Changes submitted via gerrit.. > https://git.eclipse.org/r/19366 Thanks for the sample, that demonstrates the new interface. I have a couple of general comments. Why did you set the color and font properties for the line, instead of breaking them up into Property and Value? Is there nothing that targets the Property/Value cells? In the example, it's not clear who owns what. For example, the getImage() on the IPropertySheetEntry says that callers should not dispose the image, the entry is responsible for it. Looking at IColorProvider and IFontProvider, they make no claims about what is responsible for the image. Based on usage patterns, I would guess that we're not allowed to dispose whatever comes from the IColorPovider, but it's not clear when those resources would be disposed (I guess the LabelProvider should clear up what it owns). PW
That the entry provides the style attributes like colors and font, seems wrong to me. I'd rather see a solution where a label provider (or decorator) can be contributed that implements IFontProvider and/or IColorProvider.
Paul, The main reason for styling the line instead of cells, was because that's all I needed and it's likely to be the most common use case. I had considered (even implemented) support for ITableFontProvider and ITableColorProvider, which works as one would expect. This naturally lead to wanting to support ITableLabelProvider to allow an image to be placed in either column, but it also allows the text to be set for either column (or requires a function that isn't used). As this was going beyond what I needed and I wasn't entirely happy with this potential for problems, it seemed sensible to restrict support to the simpler use case. On the ownership front, the new calls should probably have the same warning as getImage as I would expect the respective provider to manage those objects (or to have delegated that elsewhere, e.g. ResourceManager)
Dani, While the changes occur within the PropertyEntry, the colors and fonts are obtained from the LabelProvider set on the PropertyDescriptor (if it implements the interfaces).
(In reply to Kevin Milburn from comment #9) > Dani, > > While the changes occur within the PropertyEntry, the colors and fonts are > obtained from the LabelProvider set on the PropertyDescriptor (if it > implements the interfaces). I see. Maybe it's better if we just provide API for the editValue and for the label provider and let the viewer use what it wants instead of duplicating all the getter methods in the entry.
(In reply to Dani Megert from comment #10) > (In reply to Kevin Milburn from comment #9) > > Dani, > > > > While the changes occur within the PropertyEntry, the colors and fonts are > > obtained from the LabelProvider set on the PropertyDescriptor (if it > > implements the interfaces). > > I see. Maybe it's better if we just provide API for the editValue and for > the label provider and let the viewer use what it wants instead of > duplicating all the getter methods in the entry. Another thing to consider, is what is actually passed to the get methods. My current implementation passes the entry itself as it was more useful than the value. It would have been useful in the getImage too, but it's too late for that. One issue is where to document to the implementor of the providers what to expect. In my case I need the property id and source to determine what I want to return, which a derived entry allowed me to obtain.
Looking at this again, do we (you) really need a new interface? In the old days, PropertySheetEntry was internal, but now it's public and I don't know of any other IPropertySheetEntry implementation. So, until someone really needs color and font support for an implementation that doesn't inherit from PropertySheetEntry, I suggest we simply add the get* methods to PropertySheetEntry (protected) and in the viewer we test for PropertySheetEntry instead of IPropertySheetEntry2.
(In reply to Dani Megert from comment #12) > Looking at this again, do we (you) really need a new interface? In the old > days, PropertySheetEntry was internal, but now it's public and I don't know > of any other IPropertySheetEntry implementation. So, until someone really > needs color and font support for an implementation that doesn't inherit from > PropertySheetEntry, I suggest we simply add the get* methods to > PropertySheetEntry (protected) and in the viewer we test for > PropertySheetEntry instead of IPropertySheetEntry2. In reality, I don't need a new interface, I did that primarily to fit in with the getImage usage (and it seemed like a good idea at the time). Just having the methods within the PropertyEntry would suffice.
(In reply to Dani Megert from comment #12) > Looking at this again, do we (you) really need a new interface? In the old > days, PropertySheetEntry was internal, but now it's public and I don't know > of any other IPropertySheetEntry implementation. So, until someone really > needs color and font support for an implementation that doesn't inherit from > PropertySheetEntry, I suggest we simply add the get* methods to > PropertySheetEntry (protected) and in the viewer we test for > PropertySheetEntry instead of IPropertySheetEntry2. Paul, any objection?
(In reply to Dani Megert from comment #14) > > Paul, any objection? No objections, that sounds fine, PW
I've taken Kevin's change and made some minor tweaks: - removed the interface - removed all unrelated whitespace changes - marked the new methods protected and added Javadoc - increased bundle version to 3.7 Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=50fe15070d10491b584a27a7fa12f699a30a458d Thanks Kevin.
*** Bug 178009 has been marked as a duplicate of this bug. ***
In 4.4.0.I20140120-2000 Kevin, could you also please test that you can use the new API path? You are looking for one of the I builds: http://download.eclipse.org/eclipse/downloads/ PW
I can confirm the requested behaviour is present and functional in 4.4.0.I20140121-200 Thanks Kevin
