Bug 142655 - [Viewers] Rethink Viewer Concept of ICellModifier, columnProperties, CellEditor
Summary: [Viewers] Rethink Viewer Concept of ICellModifier, columnProperties, CellEditor
Status: RESOLVED DUPLICATE of bug 151295
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.3   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard: haspatch
Keywords: investigate
: 75114 75557 (view as bug list)
Depends on: 142654 149193
Blocks: 144111 144114 144115
  Show dependency tree
 
Reported: 2006-05-18 18:00 EDT by Thomas Schindl CLA
Modified: 2007-01-05 10:29 EST (History)
13 users (show)

See Also:


Attachments
Complete reimplementation for TreeViewer and TableViewer (32.64 KB, application/zip)
2006-05-18 18:08 EDT, Thomas Schindl CLA
no flags Details
Refactored to make tab-traversing more intelligent (32.33 KB, application/zip)
2006-05-18 18:42 EDT, Thomas Schindl CLA
no flags Details
Path for Viewers to add new editing support (80.73 KB, patch)
2006-05-19 13:51 EDT, Thomas Schindl CLA
no flags Details | Diff
Example/Test Code to show that both APIs are working (11.36 KB, application/zip)
2006-05-19 13:54 EDT, Thomas Schindl CLA
no flags Details
Revised patch to avoid subclassing of SWT-Widget (95.76 KB, patch)
2006-05-25 08:30 EDT, Thomas Schindl CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2006-05-18 18:00:20 EDT
I think the concept of ICellModifier is not really intuative. I think that all the things spread about those many classes can be handled from TableColum/TreeColumn in an easier way.
Comment 1 Thomas Schindl CLA 2006-05-18 18:08:10 EDT
Created attachment 41958 [details]
Complete reimplementation for TreeViewer and TableViewer

TableViewer and TreeViewer have to be reimplemented in my case because sub-classing does not provide access to treeViewerImpl/tableViewerImpl.
Comment 2 Thomas Schindl CLA 2006-05-18 18:42:00 EDT
Created attachment 41965 [details]
Refactored to make tab-traversing more intelligent

TabTraversing is now applied to the currently active editor and removed when it is closed. The rest stays as is.
Comment 3 Boris Bokowski CLA 2006-05-18 20:19:12 EDT
I agree that an abstraction for columns is needed. However, I would like to add the column abstraction to TableViewer and TreeViewer instead of copying the code.
Comment 4 Boris Bokowski CLA 2006-05-18 20:25:41 EDT
While doing this, we should add support for element-specific cell editors (there is a bug for this but I cannot find it right now).

Marking as 3.3.
Comment 5 Thomas Schindl CLA 2006-05-18 20:36:49 EDT
Well in my proposal I only needed to copy the viewers because you I can not get access to tableViewerImpl/treeViewerImpl the whole magic is done in TableEditor/TreeEditor and the TreeColumn/TableColumn. If you studied the example you don't need to duplicate too much of your code if you use the IMultiColum-Interface. The best about this it could be made backward compatible by deciding internally if the old TableEditor or the new one should be used.

If you want me to I can send you a patch which incooperates the whole into existing TreeViewer/TableViewer.

My idea is the following:
org.eclipse.jface.viewers.TableColumn/TreeColumn (abstract-classes):
  - Registers the CellEditors needed for the various object types you have (nowadays Table/TreeViewer.setCellEditors())
  - provides the values for the editor (nowadays ICellModifier)
  - updates the object from the editor (nowadays ICellModifier)

IMultiColumn:
  - interface if you implement all your logic into one concrete Table/TreeColumn
    (see TableViewer in my example)

This approach has in my ideas the following advantages:
  - multiple celleditors in one column based upon the object you are editing
  - IMultiColumn if you have say only columns dealing with TextCellEditor you
    can have the famous already known column-index switch-case from 
    LableProvider
Comment 6 Thomas Schindl CLA 2006-05-18 20:38:22 EDT
(In reply to comment #4)
> While doing this, we should add support for element-specific cell editors
> (there is a bug for this but I cannot find it right now).
> 
> Marking as 3.3.
> 

See my example it's working already ;-)
Comment 7 Thomas Schindl CLA 2006-05-18 20:49:55 EDT
(In reply to comment #6)
> (In reply to comment #4)
> > While doing this, we should add support for element-specific cell editors
> > (there is a bug for this but I cannot find it right now).
> > 
> > Marking as 3.3.
> > 
> 
> See my example it's working already ;-)
> 
and this implementation already incoopertates tab-traversing in a bit more elegant way than described in bug #75114 

Comment 8 Boris Bokowski CLA 2006-05-19 07:37:14 EDT
*** Bug 75114 has been marked as a duplicate of this bug. ***
Comment 9 Thomas Schindl CLA 2006-05-19 13:51:23 EDT
Created attachment 42048 [details]
Path for Viewers to add new editing support

adds new interfaces for Viewers (TreeViewer/TableViewer):
  - multiple cell-editors per column based upon current model (IEditableColumn)
  - support for ICellModifier like Column-Implementations (IEditableMultiColumn)
  - Tab-Traversal support for both APIs
The whole system is backward compatible the old style of setting CellEditors, ColumnProperties, ICellModifier is working as it has in the past but I've marked them as deprecated which might be too strict
Comment 10 Thomas Schindl CLA 2006-05-19 13:54:39 EDT
Created attachment 42050 [details]
Example/Test Code to show that both APIs are working
Comment 11 Boris Bokowski CLA 2006-05-19 14:04:32 EDT
Thank you for the patch! I don't have time right now to review it, but I will come back to this early in the 3.3 cycle.
Comment 12 Thomas Schindl CLA 2006-05-19 15:28:02 EDT
(In reply to comment #11)
> Thank you for the patch! I don't have time right now to review it, but I will
> come back to this early in the 3.3 cycle.
> 

Thanks. Are the any other interesting areas in JFace I could dive into and produce patches, hopefully not all that big than this one?
Comment 13 Boris Bokowski CLA 2006-05-21 11:30:59 EDT
*** Bug 75557 has been marked as a duplicate of this bug. ***
Comment 14 Boris Bokowski CLA 2006-05-21 11:31:50 EDT
> *** Bug 75557 has been marked as a duplicate of this bug. ***

Note that there is an attachment to bug 75557 that should be looked at as well.
Comment 15 Thomas Schindl CLA 2006-05-25 08:30:32 EDT
Created attachment 42561 [details]
Revised patch to avoid subclassing of SWT-Widget

I rethought the last patch which had the problem that it sub-classed TableColumn and TreeColumn which is not following the SWT-Paradigm and had the draw-back that one could not add support for editing to existing TreeColumns/TableColumns.

It holds holds the patch for bug #83200 but it's not hard to remove it when it is not accepted
Comment 16 Brad Reynolds CLA 2006-06-20 09:30:53 EDT
(In reply to comment #4)
> While doing this, we should add support for element-specific cell editors
> (there is a bug for this but I cannot find it right now).

I believe this is in bug 70800 (TableViewer) and bug 113713 (TreeViewer).
Comment 17 Thomas Schindl CLA 2006-08-01 07:31:30 EDT
Maybe this bug could be closed and all discussion move to bug 149193 which tries to invent a new concept for viewers.
Comment 18 Thomas Schindl CLA 2006-10-20 08:10:19 EDT
I think we could close this bug because it has no need any more:
- Tabediting is discussed and implemented in bug #151295
- CellEditors-Activation is discussed and implemented in bug #87733

I think the bugs 144111 144114 144115 could be moved to bug #151295 although 151295 goes beyond what is needed for BiRT (only Tab-Editing) if I read the bug reports right
Comment 19 Thomas Schindl CLA 2007-01-05 10:22:30 EST

*** This bug has been marked as a duplicate of bug 151295 ***