Bug 171844 - [Viewers] Should the TableLayout-Data connected to the column?
Summary: [Viewers] Should the TableLayout-Data connected to the column?
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.3 M6   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 171824
  Show dependency tree
 
Reported: 2007-01-26 14:03 EST by Thomas Schindl CLA
Modified: 2007-03-23 08:53 EDT (History)
4 users (show)

See Also:

Attachments
Patch to make Layout behave like normal layout (34.74 KB, patch)
2007-03-13 18:20 EDT, Thomas Schindl CLA 		no flags Details | Diff
patch for Brad's idea (38.34 KB, patch)
2007-03-14 08:56 EDT, Thomas Schindl CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2007-01-26 14:03:29 EST 
Currently we added the layout information to the TableColumnAdapter but this is not 100% correct. It would be wiser to add this information to the TableColumn because if you set Table/TreeColumns moveable the data should move with the column. Now that we are independent from SWT we could simply add this to TableViewerColumn?
Comment 1 Brad Reynolds CLA 2007-03-13 17:40:08 EDT 
My issues with this would be:
1) The API to set the column data is available on the layout and it could be  confusing to the user and potentially error prone.  Documentation could probably clear this up enough to make it usable.
2) Not everyone is using the new viewer APIs.  So if this was added I'd prefer getColumnDatas() be added to TableLayout and AbstractColumnAdapter API[1] as well.

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=177213
Comment 2 Thomas Schindl CLA 2007-03-13 17:51:13 EDT 
I'd remove the methods from the TableColumnLayout to avoid the confusion and propagate something like this:

TableColumn#setData(AbstractColumnLayout.LAYOUT_DATA,ColumnLayoutData);

Sad enough we don't have the chance to get something like TableColumn#setLayoutData(Object)
Comment 3 Thomas Schindl CLA 2007-03-13 18:20:20 EDT 
Created attachment 60740 [details]
Patch to make Layout behave like normal layout

I think this is the better solution than to add the layout data directly to the layout using addLayout we have in bug #171824 before sending #171824 out for PMC we should think whether we would like to go this way because it has the advantage that columns could be moved easily.
Comment 4 Brad Reynolds CLA 2007-03-13 19:42:43 EDT 
Tom, how about something like...

layout.setColumnData(column, new ColumnWeightData(5));

Behind the scenes you could use the data attribute but it would hide that from the consumer.  Using setData(...) is not very discoverable and is difficult to document.
Comment 5 Tod Creasey CLA 2007-03-13 21:02:47 EDT 
I agree with Brad. However I am happy with our original solution proposed in Bug 171824
Comment 6 Thomas Schindl CLA 2007-03-14 08:56:48 EDT 
Created attachment 60793 [details]
patch for Brad's idea
Comment 7 Thomas Schindl CLA 2007-03-19 11:02:31 EDT 
Fixed by resolving bug >= 20070319
Comment 8 Thomas Schindl CLA 2007-03-19 11:03:05 EDT 
once more Fixed by resolving bug >= 20070319
Comment 9 Thomas Schindl CLA 2007-03-19 11:06:20 EDT 
The bug resolving this one was bug #171824
Comment 10 Tod Creasey CLA 2007-03-23 08:53:09 EDT 
Verified by verification for Bug 171824