Bug 186800 - [Trim] Provide public API to re-layout Trim contents
Summary: [Trim] Provide public API to re-layout Trim contents
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows NT
: P3 enhancement with 1 vote (vote)
Target Milestone: 3.4   Edit
Assignee: Eric Moffatt CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-14 10:47 EDT by Kris Klindworth CLA
Modified: 2019-11-10 14:33 EST (History)
8 users (show)

See Also:


Attachments
Screenshot (6.86 KB, image/png)
2007-09-10 15:20 EDT, dev2null CLA
no flags Details
WorkbenchWindowControlContribution (1.50 KB, application/octet-stream)
2007-09-10 15:22 EDT, dev2null CLA
no flags Details
Simple RCP app to demonstrate some contribution issues (9.01 KB, application/zip)
2007-09-24 11:39 EDT, Kris Klindworth CLA
no flags Details
This code updates the ToolBarManager to not rely on item counts as the only indication of 'actual' change (2.27 KB, patch)
2007-12-17 13:40 EST, Eric Moffatt CLA
no flags Details | Diff
Screenshot of expanding contributions that can be obscured by separator. (10.92 KB, image/png)
2008-02-07 09:46 EST, Kris Klindworth CLA
no flags Details
Screenshot of shrinking contributions that leave extra space. (11.43 KB, image/png)
2008-02-07 09:47 EST, Kris Klindworth CLA
no flags Details
Screenshot of expanding contribution that doesn't push others down to next row (10.73 KB, image/png)
2008-02-07 09:47 EST, Kris Klindworth CLA
no flags Details
Screenshot of what should happen when expanding contribution grows too large to fit all items in the first row. (10.85 KB, image/png)
2008-02-07 09:48 EST, Kris Klindworth CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kris Klindworth CLA 2007-05-14 10:47:45 EDT
Build ID: I20070503-1400

Steps To Reproduce:
I've added a WorkbenchWindowControlContribution to one of the trim areas using the new org.eclipse.ui.menus extension point.  It works great, however I'd like to add a dynamic control, that is one whose children change in some way that will affect the layout of the trim.

For example, the following control contribution adds a Label and a Button to the Trim area.  When the button is clicked the text of the Label is changed.  However, the control does not appear correctly in the Trim at this point because the Label text is too long for the Label itself.  

	protected Control createControl(Composite parent) {
		Composite composite = new Composite(parent, SWT.NONE);
		GridLayout layout = new GridLayout(2, false);
		layout.marginHeight = 0;
		layout.marginWidth = 0;
		composite.setLayout(layout);
		composite.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));

		final Label label = new Label(composite, SWT.NONE);
		label.setText("Label");

		Button button = new Button(composite, SWT.PUSH);
		button.setText("Add");
		button.addSelectionListener(new SelectionAdapter() {
			@Override
			public void widgetSelected(SelectionEvent e) {
				Model.label = "Something longer";
				label.setText("Something longer");

				// Need to trigger TrimLayout update here
			}
		});

		return composite;
	}

I posed this scenario on the Platform newsgroup and it doesn't seem that there is a public API to accomplish this.  Can one be exposed to support these dynamic types of control contributions?
Comment 1 Eric Moffatt CLA 2007-05-14 15:32:19 EDT
Kris, while I agree that this is desirable (even necessary) behavior it's waaaay past API freeze so I doubt that this will make it into the 3.3 release.

In the interim you might try downcasting to the 'internal' classes (WorkbenchLayout#getTrimManager returns an ITrimManager that has a 'forceLayout()' method...).
Comment 2 Kris Klindworth CLA 2007-05-14 15:50:44 EDT
Eric,

Never in my wildest dreams had I thought that this would be considered for 3.3. :) Just glad to see that this might be something available in 3.4.

Regarding the TrimManager forceLayout method, I tried that and it had no effect.  Placed below the comment (// Need to trigger TrimLayout update here) in the original snippet:

((WorkbenchWindow)getWorkbenchWindow()).getTrimManager().forceLayout(); 

Comment 3 Eric Moffatt CLA 2007-05-14 16:18:41 EDT
Hmmm, there's one there somewhere, I'll track it down for you...
Comment 4 Eric Moffatt CLA 2007-05-15 08:35:20 EDT
Kris, this is what I use in FastViewManager so I know that it works in at least my scenarios...take a look at the FVM (perhaps you have to explicitly downcast to 'TrimLayout'?).
Comment 5 dev2null CLA 2007-09-10 07:27:01 EDT
I might have run into the same problem using 3.3 stream build M20070829-0800.

I have a WorkbenchWindowControlContribution that creates a Composite with an image and label that are dynamically updated. When updating the label with a longer text the text gets truncated.

I trigger the update using getParent().update(). My contribution has overriden isDynamic() to return true. The update() seems to trigger the contribution to be removed and readded to the toolbar.

The documentation at
http://help.eclipse.org/help33/index.jsp?topic=/org.eclipse.platform.doc.isv/guide/workbench_cmd_menus.htm talks about returning the correct values for computeSize. Is there an extra step that I've missed? (computeSize get called on my composite).

The internal api 
((WorkbenchWindow)getWorkbenchWindow()).getTrimManager().forceLayout() didn't help.
Comment 6 dev2null CLA 2007-09-10 14:09:16 EDT
Could you consider changing the Severity to minor or major (if there is no workaround for this problem)?

An API for Trim re-layout would mean that the WorkbenchWindowControlContribution have to know where it's added. The api states:
 * Abstract base class from which all controls contributions to
 * the workbench through the 'org.eclipse.ui.menus' extension
 * point must derive.

The API does not say that WorkbenchWindowControlContribution  can only be added to the trim :) 

There should be a generic way for triggering a update / relayout from a WorkbenchWindowControlContribution. Is getParent().update(true) that mechanism?
Comment 7 dev2null CLA 2007-09-10 15:20:55 EDT
Created attachment 78013 [details]
Screenshot
Comment 8 dev2null CLA 2007-09-10 15:22:13 EDT
Created attachment 78014 [details]
WorkbenchWindowControlContribution
Comment 9 dev2null CLA 2007-09-10 15:30:32 EDT
A WorkbenchWindowControlContribution added to the main toolbar resizes correctly but if the contribution is added to the trim it does resize.

Steps to reproduce the problem:
1) create a simple RCP app.
2) Enable the main toolbar and the status bar, e.g. update preWindowOpen in WorkbenchWindowAdvisor:
configurer.setShowCoolBar(true);
configurer.setShowStatusLine(true);

3) Add the attached ExampleWorkbenchWindowControlContribution class to the project

4) Add the ExampleWorkbenchWindowControlContribution to the toolbar and to the status trim, e.g. 

<extension
         point="org.eclipse.ui.menus">
      <menuContribution
            locationURI="toolbar:org.eclipse.ui.main.toolbar?after=additions">
         <toolbar
               id="example.toolbarcontrib.workingtoolbar">
            <control
                  class="example.toolbarcontrib.ExampleWorkbenchWindowControlContribution">
            </control>
         </toolbar>
      </menuContribution>
      <menuContribution
            locationURI="toolbar:org.eclipse.ui.trim.status?after=BEGIN_GROUP">
         <toolbar
               id="example.toolbarcontrib.brokentoolbar">
            <control
                  class="example.toolbarcontrib.ExampleWorkbenchWindowControlContribution">
            </control>
         </toolbar>
      </menuContribution>
      
   </extension>

5) Launcg the app and repeatably hit the two add buttons.
Comment 10 Kris Klindworth CLA 2007-09-10 15:43:12 EDT
FWIW I was never able to get the ITrimManager forceLayout method to work, even when downcasting to TrimLayout.

Good tip on the use of isDynamic and getParent().update(true).  It would be nice to have a way to update the layout without re-rendering the entire control contribution though.
Comment 11 Eric Moffatt CLA 2007-09-13 09:11:12 EDT
I'm going to be replacing the layout code for the WBW this cycle and will need to provide this mechanism in order to do it (the reason that it works in the main toolbar is that it's a CoolBar with 'n' ToolBars in it and the CoolBar manages its layout when the TB's change; the Trim doesn't).

I'm not yet sure that this will end up neing API (it may be that I simply allow contributed trim to adjust its size and automatically respond to the adjustment (but I'm still kicking the idea around).
Comment 12 dev2null CLA 2007-09-14 04:08:25 EDT
I would prefer letting the "contributed trim to adjust its size and automatically respond to the adjustment" rather than adding a new public API. The WorkbenchWindowControlContribution shouldn't need to be aware of it's specific container(s). Should I open a new bug for this or can you change this severity of this bug from enhancement to major/minor.

When moving the status trim the control gets correctly resized. Is it possible to programatically force a re-layout of the trim through a dummy move as a temporary workaround?
Comment 13 Kris Klindworth CLA 2007-09-17 13:25:31 EDT
(In reply to comment #11)
> I'm going to be replacing the layout code for the WBW this cycle and will need
> to provide this mechanism in order to do it (the reason that it works in the
> main toolbar is that it's a CoolBar with 'n' ToolBars in it and the CoolBar
> manages its layout when the TB's change; the Trim doesn't). 

Eric,

Are you tracking requirements for this re-work effort somewhere public?  There are some other issues and quirky behaviors I've encountered with trim/toolbar control contributions that are layout related and I was curious if they are on your radar.
Comment 14 Eric Moffatt CLA 2007-09-17 15:48:49 EDT
Kris, If you can put them here that'd be fine by me...;-). This bug -is- on my radar and I'd be happy to take any input into account when addressing the problem(s) with the current one. If you can track down any existing defects for these I'd be most appreciative.

The most major issue I've seen so far is that the various layouts (specifically the ToolBar layout won't take a contributed control's (i.e. a SEPARATOR item with a bound control) -height- into account on anything except Windows based systems. This means that having TB's with only a single element which is a separator with a control in it then it's -zero- height. We're working on this one...;-).
Comment 15 Kris Klindworth CLA 2007-09-24 11:38:33 EDT
I've attached a simple RCP app to demonstrate some of the issues I've seen.  We've already established that Trim contributions can't be re-sized, and as dev2null pointed out, toolbar contributions can be re-sized by utilizing the isDynamic method and calling getParent().update(true).  However, there are still a few issues with this approach that I wanted to make sure are known.

1. Expanding a toolbar contribution works, but the toolbar containing the contribution doesn't expand with it.  Using the example app, click the "Expand" button a view times on the first contribution in the main toolbar.  Notice that while the control itself is growing, the toolbar is not. The user has to manually expand the toolbar to see the entire control contribution. Also the user can shrink the toolbar down to the point where part of the contribution is hidden.  I think the toolbar should expand here automatically as the controls within it change size. This feels consistent with the current behavior for toolbars composed entirely of commands, as it's not possible to resize a toolbar smaller than the items inside of it.

2. Shrinking a contribution also works, but the toolbar itself can't be resized smaller than it was on initial start up. For example click the "Shrink" button a view times on the first control in the main toolbar to see it remove characters from the label.  Note that it's not possible to grab the toolbar and make it any smaller than the space reserved to display the label "Text" as it stood on initial startup.  It would be nice if the toolbar containing this contribution automatically recalculated it's minimum size and re-sized itself when it's children changed.

3. There seems to be some vertical constraints imposed on control contributions.  To illustrate this compare the first and second toolbars in the main application toolbar.  The first contribution is smaller vertically than the second.  The difference is that a command was contributed in the second toolbar.  It's important to note this only happens when the command has text in place of an icon, and with a style of "pulldown".  It seems that the inclusion of this pulldown command gives the toolbar more vertical space.  This is a problem since at times it's hard to get all controls in the global toolbar to vertically align correctly when our app toolbars are a mixture of commands and control contributions. 
 
This leads me to another question that may be appropriate to spin off as a separate enhancement.  Can the vertical constraint currently imposed on the global toolbar and trim areas be removed?  It would be nice if I could contribute anything I wanted as a control contribution and have the toolbar/trim automatically size itself in the vertical dimension to fully display the contribution.
Comment 16 Kris Klindworth CLA 2007-09-24 11:39:34 EDT
Created attachment 79080 [details]
Simple RCP app to demonstrate some contribution issues
Comment 17 Eric Moffatt CLA 2007-09-24 15:57:05 EDT
Kris, thanks for the patch...it'll make a really nice test set for whatever I do.

At least some of the 'vertical' (height) issues are due to mis-behaving code in SWT (i.e. a ToolBar with -only- a SEPARATOR and associated control will report its height as -zero- on some platforms. They are working on this (hmm, should check on how that's going...;-).
Comment 18 Eric Moffatt CLA 2007-12-17 13:40:29 EST
Created attachment 85402 [details]
This code updates the ToolBarManager to not rely on item counts as the only indication of 'actual' change


If the TBM.layout(true) is called we now ensure that the actual TB is tested to see if it has changed size, regardless of whether the item count changes.

Also, should the TB be contained in a CoolBar it also updates its CoolItem's size to match the change in its size.

Adding as a patch before committing should we decide that this isn't the way to go...
Comment 19 Eric Moffatt CLA 2007-12-17 13:48:03 EST
Committed in >20071217. Applied a (slightly modified) version of the above patch.

Kris, This appears to solve at least those issues that occur using your RCP scenarios. Let me know if you run into more issues.
Comment 20 Eric Moffatt CLA 2008-02-05 14:57:25 EST
Verified in I20080205-0010.
Comment 21 Kris Klindworth CLA 2008-02-07 09:46:03 EST
Eric,

Sorry for the late response on this, I've been meaning to try this out but it fell through the cracks...

I grabbed the integration build you verified with and tried it out with the simple RCP app attached previously and it works much better.  However, there are still a few issues. I'll add a few screen shots to help demonstrate the issues.

1. When the size of a contribution shrinks or expands, the minimum size for that item isn't being updated.  If you look at the attached shrinkContrib.png you'll see extra space between the "T" and the CoolBar separator.  I can't drag that separator to get rid of the extra space.  Also when expanding it's possible to grab the separator and obscure part of the contribution if it is bigger than it's initial size (see expandContrib.png for this one).

2. I'm pretty sure this is related to #1, but when expanding, items don't wrap to the next row of the CoolBar when they should.  If you look at expandContrib_2.png you can see part of the "About" button gets cut off as the dynamic contribution is expanded.  I would think it should wrap to the next row as shown in expandContrib_expected.png.  In fact if I decrease the horizontal size of the application shell a little bit it does jump down to the next row.

Other than that this is a big improvement from the previous behavior - nicely done!
Comment 22 Kris Klindworth CLA 2008-02-07 09:46:43 EST
Created attachment 89134 [details]
Screenshot of expanding contributions that can be obscured by separator.
Comment 23 Kris Klindworth CLA 2008-02-07 09:47:14 EST
Created attachment 89135 [details]
Screenshot of shrinking contributions that leave extra space.
Comment 24 Kris Klindworth CLA 2008-02-07 09:47:38 EST
Created attachment 89137 [details]
Screenshot of expanding contribution that doesn't push others down to next row
Comment 25 Kris Klindworth CLA 2008-02-07 09:48:11 EST
Created attachment 89138 [details]
Screenshot of what should happen when expanding contribution grows too large to fit all items in the first row.
Comment 26 Kondal Kolipaka CLA 2015-11-16 02:52:55 EST
Seems to be still there are resize/layout issues with updating the toolbar even with the Kepler.

public class MyHomeContribution extends WorkbenchWindowControlContribution
{
	@Override
	protected Control createControl(Composite parent)
	{
		Composite composite = new Composite(parent, SWT.NONE);
		GridLayout compositeLayout = new GridLayout(1, false);
		compositeLayout.marginHeight = 0;
		compositeLayout.marginWidth = 0;
		composite.setLayout(compositeLayout);

		final Label text = new Label(composite, SWT.NONE);
		text.setText("Eclipse");
		text.addMouseListener(new MouseListener()
		{
			public void mouseUp(MouseEvent e){}
			public void mouseDown(MouseEvent e){}

			public void mouseDoubleClick(MouseEvent e)
			{
				text.setText("Got changed to Eclispe Kepler...");
				refresh(text);
			}
		});
		return composite;
	}
	protected void refresh(final Label text)
	{
		text.getParent().update(); //doen't work
		text.getParent().layout(true); //no impact
	}
	@Override
	public boolean isDynamic()
	{
		return true;
	}
}

Expected: When I double click on the text "Eclipse" should change to "Got changed to Eclispe Kepler...". But it only shows "Got changed to", the remaining text is not appearing.

Is there any other way i can achieve this?