Community
Participate
Working Groups
N20070127-0010 (this is in code I just committed to HEAD) If you open the new 'Hyperlinking' preference page (just type that in as keyword to find the page) and resize it a lot of flickering goes on in the UI. I'm using the new TableColumnAdapter to do the table resizing.
Maybe we need somewhere setRedraw()-calls but the problem is that we don't know when the resizing is over. Does the flickering result from makeing the Table-Scrollbars visible when resizing or is it just the TableColumns?
Maybe should add some delay and wait if another resize event occurrs within a certain amount of time before resizing the columns?
I have test some code but it always comes to the point that the resize the scrollbars are shown before we get to resize our columns. The only possibility we have is to leave some extra space on the right of the table e.g. 10px and then the scrollbar only shows up short if you resize very fast. I don't have any other clue how we could manage to resize the columns before the widget shows up the scrollbars. To give it a try you can use add the following line to AbstractColumnAdapter#controlResized() "width -= 10;" maybe we should at least provide the users the possibility to provide a right-padding?
You could take a look at the editor template preference page code (TemplatePreferencePage). There's much less flickering when resizing that page. The relevant part is in ColumnLayout.
Created attachment 57876 [details] Revised Table/TreeColumn Layout Ok. That's nice and really completely removes the flickering of the horizontal scrollbar. The drawback is that this works a little different than the what we've done before so we may break existing implementors. Even if this code doesn't make it into JFace you could maybe patch your ColumnLayout implementation which has a problem when the Table pops up starting with no scrollbars. The relevant bits are: protected void layout(Composite composite, boolean flushCache) { // For the first time we need to relayout because Scrollbars are not calculate appropiately if( composite.getData(RECALCULATE_LAYOUT) == null ) { composite.setData(RECALCULATE_LAYOUT,Boolean.FALSE); composite.layout(); } private int computeTrim(Rectangle area, Scrollable table, int tableWidth) { // ... if (!vBar.isVisible() ) { Point vBarSize= vBar.getSize(); trim += vBarSize.x; }
Thanks Tom. I'd like to see a public solution from JFace. The code has been introduced during 3.3 and we can still break such API for M5. At least in the SDK it seems that I am the only client.
I know I'm the one to blame because I introduced the code. At the time nobody pointed me to ColumnLayout but to TableColumnLayout which is also part of JDT-UI I think. I'm also +1 for the current layout solution the only thing I think be a great thing to have is described in bug #171844 because the user currently needs to write custom code when TableColumns are moveable. But I didn't get any feedback from our UI-Gurus, I think they are all loaded with a lot of work. M5 is really near and I don't know how can manage to get all my API changes in but this certainly is a really really import change we definately need to make in before M5. Boris, Tod any comments from your side?
A one more thing just came to my mind currently all our layout code has the problem that the Table/Tree has to be the only container. Wouldn't it make sense to provide dedicated widgets and not expose the layout classes. This way we would have: TableLayoutComposite talc = new TableLayoutComposite(parent, SWT.FULL_SELECTION); talc.moveLayoutDataWithColumn(true); // control if the layout-data is connected talc.addLayoutData(...); TreeLayoutComposite trlc = new TreeLayoutComposite(parent,SWT.FULL_SELECTION); This way we make it clear that this is a dedicated composite for a table/tree we could even throw an exception when some one tries to set a layout on this composite from external! Currently I always find myself write code like this: Composite realContainer = .... Composite layoutHelper = new Composite(parent,SWT.NONE); TableViewer viewer = new TableViewer(layoutHelper); TableColumnLayout lc = new TableColumnLayout(); layoutHelper.setLayout(lc); The great thing with this is that it works also for Trees and bug #164038 is resolved.
This bug looks like a duplicate of bug #89171
This is a refactored class from org.eclipse.ui.texteditor.templates.ColumnLayout because I couldn't find it any more yesterday ;-)
This patch is much better because it is a proper SWT Layout which is a much better practise. A couple of things to mention /* * @see org.eclipse.swt.widgets.Layout#computeSize(org.eclipse.swt.widgets.Composite, int, int, boolean) */ protected Point computeSize(Composite composite, int wHint, int hHint, boolean flushCache) { System.err.println("COMPUTE SIZE"); //$NON-NLS-1$ return computeTableTreeSize(getTableTree(composite), wHint, hHint); } We need to check for calls to System.err.println -getTableTree should be getControl(). References to TableTree even if accidental must be avoided - we need to really test this as there are cases where we might lose resize events. Worse case is a bad layout though so I think this is low risk.
Created attachment 58254 [details] Address Tod's issues
Do we have a snippet for testing this on the different platforms?
There are Snippet Snippet016 and a new Snippet027 I think to check the layout. Is this enough, they should be included in the patch!
We will be replacing TableColumnAdapter and related classes for M6. I added a comment to that effect to the Javadoc and marked the classes as deprecated.
Boris, I still use that class hence we should coordinate when it gets replaced. Best it probably after eclipseCon.
Is everybody happy with Tom's latest patch? We need to introduce the new API this week.
Tom this patch is out of date - would it be possible to refresh it? I am afraid of missing something if I do it manually.
Created attachment 60729 [details] updated for head
Tod, Boris, Daniel before we send this patch to the PMC I'd like to discuss the solution for connecting layout data in bug #171844 instead of our current solution.
Created attachment 60741 [details] Patch to remove all classes not needed anymore This patch only cleans up correctly the patch in bug #171844 is not part of this patch!
+1 on this patch over the solution for Bug 171844 Lines like column.setData(AbstractColumnLayout.LAYOUT_DATA, new ColumnWeightData(50, 100)); exposes our use of the data for storing JFace information which is not API anywhere else. I think this would be very confusing to our consumers. It would also reduce the flexibility in the future for us to potentially rework our internal mapping to SWT should we decide for a r0ute other than the data. We can discuss on March 14. Please let me know a good time for you on IRC but right now I think this is the way we want to go.
I agree with Tod.
Please coordinate committing the patch with me as Platform Text will be broken.
Thanks for the help again Dani. I will add the code this week but I won't delete anything until I have the go ahead from you. API request to follow.
API REQUEST Delete the experimental deprecated class Table Adapter and replace it's functionality with the more SWT AbstractColumnLayout Background: We required a class that could keep the relative sizes of columns constant during a resize so as to avoid the user resizing all of the columns of a table. In M4 we implemented the TableAdapter which was a subclass of ControlAdapter to handle this. This turned out to cause a lot of flash so we deprecated it in M5 in order to come up with a Layout based solution. Add: AbstractColumnLayout TableColumnLayout - layout class for tables TreeColumnLayout - layout class for trees Remove: (all were added in 3.3, deprecated in M5 and experimental) AbstractColumnAdapter TreeColumnAdapter TableColumnAdpter RISKS: Teams reliant on the old API will have to port. JDT Text is the only known consumer, RISKS OF NOT TAKING THIS CHANGE The ColumnAdapters are too flashy to be useful. This is a common use case that will be used within the SDK.
added bug #164038 because this would be fixed automatically if this request is accepted
Created attachment 60788 [details] only make things API who really need to
uups this give compile errors in test suite, patch to follow
Created attachment 60790 [details] fix test suite
Created attachment 60794 [details] Patch with some more commenting Here is a patch that adds some more comments to the private methods
Updated API REQUEST (made the abstract class package visible) Delete the experimental deprecated class Table Adapter and replace it's functionality with the more SWT AbstractColumnLayout Background: We required a class that could keep the relative sizes of columns constant during a resize so as to avoid the user resizing all of the columns of a table. In M4 we implemented the TableAdapter which was a subclass of ControlAdapter to handle this. This turned out to cause a lot of flash so we deprecated it in M5 in order to come up with a Layout based solution. Add: TableColumnLayout - layout class for tables TreeColumnLayout - layout class for trees Remove: (all were added in 3.3, deprecated in M5 and experimental) AbstractColumnAdapter TreeColumnAdapter TableColumnAdpter RISKS: Teams reliant on the old API will have to port. JDT Text is the only known consumer, RISKS OF NOT TAKING THIS CHANGE The ColumnAdapters are too flashy to be useful. This is a common use case that will be used within the SDK.
+1. Do you need to provide a patch for JDT Text code?
The patch I attached doesn't break JDT. I will do the deletions when Dani gives me the go ahead,
Created attachment 60837 [details] to discuss Boris I'd like to discuss this with you tommorrow because I think connecting TableColumn and LayoutData has multiple advantages (e.g. deleting columns, adding columns, ...). Deleting columns might provide problems with the current implementation.
Mike as said we'd like to request approval to change the introduced API once more?
+1
Patch ("to discuss") released >20070316. Leaving the bug open for now. If you resize one of the columns, the other columns are not updated. Tom plans to look at this on the weekend.
Created attachment 61215 [details] Patch to make update columns after one is resized Boris waiting for a comment from you until tomorrow in the morning else I'll commit it as attached here. Maybe even Dani can give comment on this too 'til then.
This seems not to work. I converted my code to use the TableColumnLayout but I get the exception listed below. Maybe it's because I (re-) use the following column weight: new ColumnWeightData(1)? !ENTRY org.eclipse.ui 4 0 2007-03-19 09:18:07.934 !MESSAGE Unhandled event loop exception !STACK 0 java.lang.ArrayIndexOutOfBoundsException: 0 at org.eclipse.jface.layout.AbstractColumnLayout.getControl(AbstractColumnLayout.java:294) at org.eclipse.jface.layout.AbstractColumnLayout.layout(AbstractColumnLayout.java:232) at org.eclipse.swt.widgets.Composite.updateLayout(Composite.java:999)
Created attachment 61257 [details] My coverted code
Dani you are assigning the layout to the wrong Component. To make this all work you need assign the layout to the artificial component. In your case I think you need to assign the layout to the parent of the Table which has to have exactly one child namely the Table. This has not changed from the implementation we copied from you. I haven't looked thoroughly at your code but I'd say it has to look something like this: --------------8<-------------- Composite layoutHelper = new Composite(editorComposite,SWT.NONE); Table hyperlinkDetectorTable = new Table(editorComposite,....); TableColumnLayout tableColumnLayout = new TableColumnLayout(); layoutHelper.setLayout(tableColumnLayout); --------------8<-------------- Maybe you can even use the editorComposite if it only has one child. The reuse of the Layout-Object has no influence (currently) although this is not consitent with other layouts where data objects *can not* be reused I'm not sure but maybe this will bring you into trouble some day so I'd suggest that you don't reuse them.
Mhh. looks a bit strange that I can't set the layout to the table itself.
Well I think we have no other possibility. This is the only way and as said it's like you did it in former days with your internal class. The layout needs to be on the parent else we'll see the flickering as we had it with the control-adapter.
k. but we should improve the Javadoc saying that this is intended for a Composite that contains the table/tree viewer.
I have a locally changed version where I already updated the javadoc like this: ----------------8<---------------- <p> <b>You can only add the {@link Layout} to a container whose <i>only</i> child is the {@link Table} control you want the {@link Layout} applied to. Don't assign the layout directly the {@link Table}</b> </p> ----------------8<----------------
I've converted the Platform Text code and released it for I20070319-0800. You can delete the *TableColumnAdapter classes.
Released >= 20070319
Verified by code inspection in I20070321-1800
I'm experimenting with adopting this new support in Jazz. I've got a view which adds/removes a column based on preferences. Using the new support, when I add the new column it is initially shown blank (no header text, no text for elements in the column) until I resize the column. This is a simplified version of the code. This worked fine before I adopted it to using the TreeColumnLayout support. Am I doing something wrong or does this new support have a problem when columns are added dynamically? private TreeViewer fViewer; private TreeColumnLayout fTreeColumnLayout; private TreeColumn fFirstColumn; public void createPartControl(Composite parent) { Composite treeComposite = new Composite(parent, SWT.NONE); fTreeColumnLayout = new TreeColumnLayout(); treeComposite.setLayout(fTreeColumnLayout); treeComposite.setLayoutData(new GridData(GridData.FILL_BOTH)); fViewer = new TreeViewer(treeComposite, SWT.H_SCROLL | SWT.V_SCROLL | SWT.MULTI); Tree tree = fViewer.getTree(); tree.setLayoutData(new GridData(GridData.FILL_BOTH)); fFirstColumn = new TreeColumn(tree, SWT.NONE); updateColumns(); } private void updateColumns() { boolean show = getColumnVisiblePreference(); Tree tree = fViewer.getTree(); tree.setHeaderVisible(show); tree.setLinesVisible(show); if (show) { fTreeColumnLayout.setColumnData(fFirstColumn, new ColumnWeightData(80, 20, true)); TreeColumn column = new TreeColumn(tree, SWT.RIGHT); fTreeColumnLayout.setColumnData(column, new ColumnWeightData(20, 10, true)); column.setText("Header"); } else { fTreeColumnLayout.setColumnData(fFirstColumn, new ColumnWeightData(100, 20, true)); if (tree.getColumnCount() > 1) { tree.getColumn(1).dispose(); } } tree.layout(); } public void propertyChange(PropertyChangeEvent event) { if (isColumnVisibleChange(event)) { updateColumns(); } }
From your code I can't see any problem. Did you tried to call treeViewer#refresh() after adding the column? If this doesn't cure the problem can you please - Create a snippet we can run to see the problem as a base you can use one of our collection (http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.jface.snippets/Eclipse%20JFace%20Snippets/org/eclipse/jface/snippets/viewers/) - File a new bug for this
Is there any reason that the new TableColumnLayout class should not work in a 3.2 Eclipse? I tried copying the code for TableColumnLayout and its parent, AbstractColumnLayout, into classes of my own for use in an Eclipse 3.2-based application. However, the results I'm getting are very strange. I was just wondering if these new layouts depend on changes to any other classes that I might have overlooked.
There were some bugfixes in SWT that this relies on. Can you use SWT 3.3 while keeping everything else at 3.2?
