Bugzilla – Full Text Bug Listing
| Summary: | [JFace] JFace does not reflect the new color options for SWT Table and tree | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Gunnar Ahlberg <gunnar> | ||||||||
| Component: | UI | Assignee: | Tod Creasey <Tod_Creasey> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P2 | CC: | danjou, eclipse, jean-michel_lemieux, john.ruud, michaelvanmeekeren, n.a.edgar, patmc | ||||||||
| Version: | 3.0 | Keywords: | helpwanted | ||||||||
| Target Milestone: | 3.1 M2 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 54534 | ||||||||||
| Attachments: |
|
||||||||||
This is unlikely for 3.0 as the API freeze occured 2 weeks ago. However we should look at this early in 3.0.1. Marking later Reopening now that 3.0 has shipped Adding Nick to the list as he is the original author. I am not clear as to why this API would be required as you can already determine the color of an cell using the IColorProvider using getForeground(Object element) and getBackground (Object element). I am not sure why you need API that includes indices as well. I need the viewer to call the IColorProvider per cell, not per row. Example: --------------------------------- | Column1 | Column 2 | Column 3 | --------------------------------- | Blue | Red | Green | --------------------------------- SWT allows this with 3.0, but as long as the viewer is calling the IColorProvider per row and not per cell, this is not possible (without modifying the lowlevel widget) Right - so an ITableColorProvider would be OK then? I don't think this API would be worth having on TreeViewers and so adding a second interface would allow us to do this in a non-breaking way. That would be consistent with ITableLabelProvider. Ah, yes, that would be the intellegent and elegant solution and would fit my needs. Gunnar if this is important to you if you would like to submit a patch with your suggested changes we could review it and make sure that it satisifes everyone before we proceed. Created attachment 12979 [details]
Patch for column based colors for TableViewers
Initial version, testcase comming
Created attachment 12980 [details]
Standalone class displaying the benefits of a ITableColorProvider
This is not a JUnit testcase but a standalone class. It will display a table
with two columns, first with red background and green foreground, and a second
column with green background and red foreground
Would you be able to provide a JUnit test as well<grin>? Created attachment 12981 [details]
Testcase for ITableColorProvider
This is a testcase that can be run in the org.eclipse.ui.tests project.
Sourcefolder: Eclipse JFace Tests
Parent class: StructuredViewerTest
Not too sure about how valid or thorough the tests are, may need some
additional work (currently there are 4)
*** Bug 57154 has been marked as a duplicate of this bug. *** *** Bug 52963 has been marked as a duplicate of this bug. *** *** Bug 54982 has been marked as a duplicate of this bug. *** The basic implementation iof ITableColorProvider looks fine - I will have to change to copyright to fit the format of the rest of Eclipse. The only change to code I will make is that your patch would mean that TableViewer would no longer support IColorProvider - this would be a breaking API change which I don't think we want to make as it is still potentially useful to highlight an entire line. As with ILabelProvider/ITableLabelProvider, the table viewers should still handle the case where an IColorProvider is given instead of an ITableColorProvider. In this case, it should apply the color to all columns (unlike ILabelProvider, which provides the text for the first column only). Patch and test released. I have not removed the reference to IColorProvider as the patch does but otherwise the code is pretty much the same. I changed the copyright to be consistent with our other contributor copyrights (with attributions of course). I have also tidied up the test case and added it to our suites. Thanks for all of the help Gunnar! As this was a small change it may be worth putting into 3.0.1. 3.0.1 is for critical fixes only, not new features or enhancements. Basically the same criteria as the 3.0 end-game. Changes similar to following would make my property sheet life a lot easier: The following viewers should support ITableColorProvider: TableViewer TableTreeViewer PropertySheetViewer They should also support something like an 'ITableFontProvider' (similar to ITableColorProvider, but for fonts). A 'DecoratingLabelProvider' if often used for wrapping the real label providers, so the viewers would also need to take that into consideration. 'PropertySheetViewer' is different from the other viewers in that it is using
an IPropertySheetEntry for rendering each item (but the entry will typically
get the values from the entry's label provider).
PropertySheetViewer renders the following values (in column 0 and 1) using the
IPropertySheetEntry:
Text: getDisplayName(), getValueAsString()
Image: NONE, getImage()
Font: NONE, NONE
The DEFAULT implementation (PropertySheetEntry) is using the entry's label
provider:
Text: descriptor.getDisplayName(), descriptor.getLabelProvider().getText()
Image: NONE, descriptor.getLabelProvider().getImage()
Font: NONE, NONE
I believe 'IPropertySheetEntry' may have to be fattened up in order to provide
the missing values (i.e. font for column 0). Alternatively, in our current
implementation the viewer delegates the rendering of each item/widget to
an 'item decorator' as follows:
PropertySheetEntry.updateEntry(IPropertySheetEntry entry, TableTreeItem item) {
...
if (entry instanceof IItemDecorator)
((IItemDecorator) entry).decorate(entry, item);
...
}
public interface IItemDecorator {
void decorate(Object object, Widget item);
}
Hi I submitted the patch for ITableColorProvider. The only thing left to touch is the contributor information. Due to legal issues, please change the attached lines Index: src/org/eclipse/jface/viewers/ITableColorProvider.java =================================================================== RCS file: /home/eclipse/org.eclipse.jface/src/org/eclipse/jface/viewers/ITableColorProvider.java,v retrieving revision 1.2 diff -u -r1.2 ITableColorProvider.java --- src/org/eclipse/jface/viewers/ITableColorProvider.java 8 Jul 2004 20:07:08 -0000 1.2 +++ src/org/eclipse/jface/viewers/ITableColorProvider.java 24 Aug 2004 07:59:56 -0000 @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/cpl-v10.html * * Contributors: - * Initial implementation - Gunnar Ahlberg - www.gunnarahlberg.com + * Initial implementation - Gunnar Ahlberg - IBS AB, www.ibs.net * IBM Corporation - further revisions *******************************************************************************/ Index: Eclipse JFace Tests/org/eclipse/jface/tests/viewers/TableColorProviderTest.java =================================================================== RCS file: /home/eclipse/org.eclipse.ui.tests/Eclipse JFace Tests/org/eclipse/jface/tests/viewers/TableColorProviderTest.java,v retrieving revision 1.2 diff -u -r1.2 TableColorProviderTest.java --- Eclipse JFace Tests/org/eclipse/jface/tests/viewers/TableColorProviderTest.java 8 Jul 2004 20:03:53 -0000 1.2 +++ Eclipse JFace Tests/org/eclipse/jface/tests/viewers/TableColorProviderTest.java 24 Aug 2004 08:01:59 -0000 @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/cpl-v10.html * * Contributors: - * Initial implementation - Gunnar Ahlberg - www.gunnarahlberg.com + * Initial implementation - Gunnar Ahlberg - IBS AB, www.ibs.net * IBM Corporation - further revisions *******************************************************************************/ Reopening to fix up comments. I have released your requested updates for build >20040825 Several viewer-provider related bugs have been marked as dupes of this bug (and are therefore closed), including: https://bugs.eclipse.org/bugs/show_bug.cgi?id=52963 TableViewer now supports ITableColorProvider, but TableTreeViewer still doesn't (I think it should). Also, it would be very useful if table viewers were to support a 'ITableFontProvider', probably modeled after ITableColorProvider. Should these be submitted as new bugs (since this one is now closed), or should discussion continue here? Please do. It is much harder for us to track progress if there are multiple problems reported in the same bug. This is particulary timely as I am looking into all of the places that JFace does not adequately support the 3.0 SWT API now. BTW I have logged a PR for and ITableFontProvider as well. See Bug 72615 Will do (I have entered BR# 72720) Verified in 200409210800 |
The IColorProvider can be implemented by a label provider to provide background and foreground colors to a viewer. This should be changed to set the colors for individual cells, as is possible with the new SWT functionality already implemented in SWT. Changes: 1) IColorProvider - two supplementing methods Color getForeground(Object element, int index); Color getBackground(Object element, int index); 2) References to IColorProvider to the new methods doUpdateItem(Item, Object) - org.eclipse.jface.viewers.TableTreeViewer (3 matches) doUpdateItem(Item, Object) - org.eclipse.jface.viewers.TreeViewer (3 matches) doUpdateItem(Widget, Object, boolean) - org.eclipse.jface.viewers.TableViewer (3 matches)