Bug 213645 - [RCP] Requesting programmatic hook to create view folders that remain visible after their last child is closed
Summary: [RCP] Requesting programmatic hook to create view folders that remain visible...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 enhancement with 2 votes (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-12-20 16:45 EST by Carlos Devoto CLA
Modified: 2011-08-24 13:08 EDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (24.37 KB, text/plain)
2007-12-20 16:55 EST, Carlos Devoto CLA
no flags Details
Updated patch (26.93 KB, text/plain)
2008-01-02 16:54 EST, Carlos Devoto CLA
bokowski: iplog+
Details
patch (12.78 KB, patch)
2009-03-08 22:34 EDT, Boris Bokowski CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Devoto CLA 2007-12-20 16:45:02 EST
Build ID: M20071023-1652

Steps To Reproduce:
1. In my perspective factory, I create a view folder using the IPageLayout.createFolder method.
2. I add a view to the folder that I created in step 1, either programmatically or through the UI.
3. I close the view that I added to the folder, either programmatically or through the UI.
4. The folder disappears.

At the developer's option, it should be possible to create a view folder that can survive the loss of its children!  Here is a test case and some helper classes that I created.  It is assumed that the StandardFolder perspective factory is registered as an extension via the plugin.xml file.

CreateFolderTest.java
---------------------------------------------------------
package org.eclipse.ui.tests.rcp;

import junit.framework.TestCase;

import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.IViewPart;
import org.eclipse.ui.IWorkbenchPage;
import org.eclipse.ui.IWorkbenchWindow;
import org.eclipse.ui.PartInitException;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.application.IWorkbenchWindowConfigurer;
import org.eclipse.ui.application.WorkbenchAdvisor;
import org.eclipse.ui.internal.PartPane;
import org.eclipse.ui.internal.ViewSite;
import org.eclipse.ui.internal.ViewStack;
import org.eclipse.ui.tests.rcp.util.EmptyView;
import org.eclipse.ui.tests.rcp.util.StandardFolderPerspective;
import org.eclipse.ui.tests.rcp.util.WorkbenchAdvisorObserver;

/**
 * Tests the behaviour of various IPageLayout.createFolder methods
 */
public class CreateFolderTest extends TestCase {

    public CreateFolderTest(String name) {
        super(name);
    }

    private Display display = null;

    protected void setUp() throws Exception {
        super.setUp();

        assertNull(display);
        display = PlatformUI.createDisplay();
        assertNotNull(display);
    }

    protected void tearDown() throws Exception {
        assertNotNull(display);
        if (!display.isDisposed()) {
            display.dispose();
        }
        assertTrue(display.isDisposed());

        super.tearDown();
    }

    /**
     * Regression test standard folder creation in which the folder disappears after its last child is closed
     */
    public void testCreateStandardFolder() {
        WorkbenchAdvisor wa = new WorkbenchAdvisorObserver(1) {

            public void preWindowOpen(IWorkbenchWindowConfigurer configurer) {
                super.preWindowOpen(configurer);
                configurer.setShowPerspectiveBar(false);
            }

            public void postStartup() {
                try {
                    IWorkbenchWindow window = getWorkbenchConfigurer()
                            .getWorkbench().getActiveWorkbenchWindow();
                    IWorkbenchPage page = window.getActivePage();
                    IViewPart view = page.showView(EmptyView.ID);
                    PartPane pane = ((ViewSite) view.getSite()).getPane();
                    ViewStack parent = ((ViewStack) (pane.getContainer()));
                    assertTrue(parent.getVisible());
                    page.hideView(view);
                    assertTrue(parent.getVisible());
                } catch (PartInitException e) {
                    fail(e.toString());
                }
            }
            
            public String getInitialWindowPerspectiveId() {
            	return StandardFolderPerspective.PERSP_ID;
            }
        };
        int code = PlatformUI.createAndRunWorkbench(display, wa);
        assertEquals(PlatformUI.RETURN_OK, code);
    }
}

StandardFolderPerspective.java
-----------------------------------------------------------
package org.eclipse.ui.tests.rcp.util;

import org.eclipse.ui.IFolderLayout;
import org.eclipse.ui.IPageLayout;
import org.eclipse.ui.IPerspectiveFactory;

/**
 * This perspective is used for testing the createFolder api.  It defines an initial
 * layout with an empty view folder.
 */
public class StandardFolderPerspective implements IPerspectiveFactory {

    /**
     * The perspective id for the empty perspective.
     */
    public static final String PERSP_ID = "org.eclipse.ui.tests.rcp.util.StandardFolderPerspective"; //$NON-NLS-1$

    /**
     * Constructs a new Default layout engine.
     */
    public StandardFolderPerspective() {
        super();
    }

    /**
     * Defines the initial layout for a perspective.  
     *
     * Implementors of this method may add additional views to a
     * perspective.  The perspective already contains an editor folder
     * with <code>ID = ILayoutFactory.ID_EDITORS</code>.  Add additional views
     * to the perspective in reference to the editor folder.
     *
     * This method is only called when a new perspective is created.  If
     * an old perspective is restored from a persistence file then
     * this method is not called.
     *
     * @param factory the factory used to add views to the perspective
     */
    public void createInitialLayout(IPageLayout layout) {
        IFolderLayout folder = layout.createFolder("folder", IPageLayout.LEFT, 1.0f, layout.getEditorArea());
        folder.addPlaceholder(EmptyView.ID);
    }
    
}

EmptyView.java
------------------------------------------------------------
/*******************************************************************************
 * Copyright (c) 2004, 2006 IBM Corporation and others.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *
 * Contributors:
 *     IBM Corporation - initial API and implementation
 *******************************************************************************/
package org.eclipse.ui.tests.rcp.util;

import org.eclipse.swt.SWT;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Label;
import org.eclipse.ui.part.ViewPart;

/**
 * Minimal view, for the RCP tests. 
 */
public class EmptyView extends ViewPart {

    public static final String ID = "org.eclipse.ui.tests.rcp.util.EmptyView"; //$NON-NLS-1$
    
    private Label label;
    
	public EmptyView() {
	    // do nothing
	}

	public void createPartControl(Composite parent) {
	    label = new Label(parent, SWT.NONE);
	    label.setText("Empty view");
	}

	public void setFocus() {
		label.setFocus();
	}
}


More information:
I would be happy to assist in any way that I can with the creation of a patch for this enhancement request.  Indeed, I have already created such a patch which I will submit, though I am not experienced enough with the Eclipse platform to ascertain whether my patch factors in all possible permutations.  The main snag that I encountered had to do with the minimization of empty view folders. I have dealt with this and added test cases for it.
Comment 1 Thomas Schindl CLA 2007-12-20 16:54:09 EST
In fact the ViewArea should behave like the editor area right?
Comment 2 Carlos Devoto CLA 2007-12-20 16:55:12 EST
Created attachment 85686 [details]
Proposed patch

I am willing to work with the team to modify this patch in any way deemed necessary.
Comment 3 Carlos Devoto CLA 2007-12-20 16:56:22 EST
(In reply to comment #1)
> In fact the ViewArea should behave like the editor area right?

Yes, that is exactly what we are looking for.
Comment 4 Paul Webster CLA 2007-12-21 07:49:37 EST
I'll just note that as of 3.3, you can provide special properties to your perspective folders retrieved from IPageLayout that can then be read by your presentation.

That might not be enough to make a view stack stick around (the framework might decide to close it when it's empty) but that could be used to make a specific view stack as "sticky"

PW
Comment 5 Carlos Devoto CLA 2007-12-21 12:53:42 EST
(In reply to comment #4)
> I'll just note that as of 3.3, you can provide special properties to your
> perspective folders retrieved from IPageLayout that can then be read by your
> presentation.
> That might not be enough to make a view stack stick around (the framework might
> decide to close it when it's empty) but that could be used to make a specific
> view stack as "sticky"
> PW

Thanks for the comment, Paul.  Let me try to restate what you are saying to see if I am understanding you correctly.  Are you proposing that we leverage the existing IFolderLayout.setProperty() API as a mechanism for flagging specific folders as "sticky", and then key off of this property within the patch to provide an implementation for the "sticky folder" behavior?  If so, I believe that has the merit of reusing an existing API as opposed to introducing a new one.  

In the patch that I whipped up, I introduced a new createDurableFolder() method to the IPageLayout interface, fully recognizing that there was probably a cleaner, less intrusive way of doing this.  I fully defer to the committers on this and any other design decisions.
Comment 6 Boris Bokowski CLA 2008-01-02 10:58:36 EST
Paul, does the presentation have a say at all whether or not view stacks are closed?  The patch does not seem to affect any presentation code.
Comment 7 Carlos Devoto CLA 2008-01-02 15:27:36 EST
(In reply to comment #6)
> Paul, does the presentation have a say at all whether or not view stacks are
> closed?  The patch does not seem to affect any presentation code.

While Paul can certainly provide a more authoritative answer, as the patch creator, I'd like point out that I saw no existing hooks that would allow a presentation to influence this behavior.  
Comment 8 Carlos Devoto CLA 2008-01-02 16:54:06 EST
Created attachment 86030 [details]
Updated patch

In the previous patch, I accidentally included the wrong version of the JUnit test case.  This patch expands the scope of the JUnit test case such that it now includes tests for the new functionality embedded within the patch.  More specifically, it includes tests to ensure that:

1. A view stack flagged as "durable" remains visible after its last child is closed.
2. A view stack flagged as "durable" can be minimized.
3. A view stack flagged as "durable" can be restored after it has been minimized.
Comment 9 Paul Webster CLA 2008-01-07 10:10:49 EST
(In reply to comment #4)
> 
> That might not be enough to make a view stack stick around (the framework might
> decide to close it when it's empty) but that could be used to make a specific
> view stack as "sticky"

As Carlos' patch indicates, it appears our framework closes and opens the stacks (not the perspective itself).

So while the new API (plus a published property constant) could be used to communicate the "sticky" stack to the framework as well as the presentation (the mechanism to persist across restarts comes for free) I'll defer to Boris if he feels another mechanism would be more appropriate.

PW
Comment 10 Borsos Laszlo CLA 2008-06-30 12:06:22 EDT
I needed this functionality, but I have found that unfortunately it is still not there in Ganymede.
Comment 11 Boris Bokowski CLA 2008-07-02 07:05:02 EDT
Sorry for missing this in 3.4.
Comment 12 Boris Bokowski CLA 2009-03-08 22:34:11 EDT
Created attachment 127977 [details]
patch

Found this literally at the last minute before the 3.5 API freeze. In similar cases, don't be shy and ping on the bug if it looks like I've dropped the ball. Too many balls to juggle, unfortunately...

I decided against the proposed new API on IPageLayout, and instead added a method to WorkbenchWindowAdvisor:

/**
 * Returns <code>true</code> if the given folder in the given perspective should remain visible even after all
 * parts in it have been closed by the user. The default is <code>false</code>.
 * 
 * @param perspectiveId the perspective id
 * @param folderId the folder id
 * @return <code>true</code> if the given folder should be durable
 * 
 * @since 3.5 
 */
public boolean isDurableFolder(String perspectiveId, String folderId) {
	return false;
}

This makes the new mechanism available for RCP applications but keeps the IDE application's policy of ensuring that the UI behaviour is consistent and predictable. It may be less than optimal that the new "durable" property has to be defined in a place that is separate from the perspective factory, but this is all I felt comfortable with at this late stage.
Comment 13 Boris Bokowski CLA 2009-03-08 22:39:25 EDT
The changed API made the tests obsolete. In total, the portion of Carlos' contribution that was released to CVS was roughly 130 lines of code, even though I am marking his larger patch with the iplog+ flag.

Carlos, thank you very much for what looks like a high-quality contribution. I am hoping that you can help us fix any bugs that may be raised by the new fatures... also, would you be able to contribute updated test cases, using a new bug?
Comment 14 Paul Webster CLA 2009-03-09 09:00:01 EDT
(In reply to comment #12)
> Created an attachment (id=127977) [details]
> patch

> public boolean isDurableFolder(String perspectiveId, String folderId) {
>         return false;
> }
> 
>

Why not simply check a property on the folder itself?  org.eclipse.ui.IPlaceholderFolderLayout.setProperty(String, String)

Or is it that you don't want this exposed to the general perspectives at this time (which the property would do)?

PW
Comment 15 Reto Urfer CLA 2010-07-09 08:45:59 EDT
I had the same problem and found the solution with overwriting the method public boolean isDurableFolder(String thePerspectiveId, String theFolderId) in the WorkbenchWindowAdvisor. 
It works fine but unfortunately the durable flag is not stored/restored with the Memento. The result is that after restarting the application the durable flag is lost until the perspective is resetted.
I think the simplest solution would be to overwrite the saveState(IMemento) and restoreState(IMemento) methods in the ViewStack class.
Comment 16 Thomas Schindl CLA 2011-08-24 11:29:36 EDT
(In reply to comment #15)
> I had the same problem and found the solution with overwriting the method
> public boolean isDurableFolder(String thePerspectiveId, String theFolderId) in
> the WorkbenchWindowAdvisor. 
> It works fine but unfortunately the durable flag is not stored/restored with
> the Memento. The result is that after restarting the application the durable
> flag is lost until the perspective is resetted.
> I think the simplest solution would be to overwrite the saveState(IMemento) and
> restoreState(IMemento) methods in the ViewStack class.

I'm experiencing the same problem - so as it is now this API makes no sense to me. I also don't get why this was not introduced in IPageLayout#createFolder() (it is marked as @noimplement) because I really only found out the WorkbenchWindowAdvisor-API by luck.

I think I'll file a new bug on this problem - i think we can at least fix this very easy in Eclipse 4.x
Comment 17 Thomas Schindl CLA 2011-08-24 13:08:09 EDT
I filed bug 355750 to fix the save and restore