Initial contribution of property-view extension

[ndoschek]

Introduction

Many IDEs, such as the traditional Eclipse IDE, have the notion of a global, extensible property view, which shows additional information about the current selection within the IDE. Property views are heavily used in those IDEs for showing details of elements in e.g. diagram editors, complex tree editors, or the file explorer selection. Therefore, the main idea is to have a global, generic property view in the IDE, but allow specific implementations to extend the contents of the global property view with specific additional information for some type of selection.

What it does

The @theia/property-view extension contributes a generic, global property view based on Theia's global selection.
The property view widget can be opened/toggled either via menu View->Properties or via shortcut Shift+Alt+P. It is located in the bottom dock area per default.
The following two default content widgets are implemented in this extension:

• EmptyPropertyViewWidget: If no other widget can be provided, a simple message (No properties available) is shown.
• ResourcePropertyViewWidget: Can handle file and monaco editor selections and displays the general FileStat information (e.g. location, name, last modified) in a TreeWidget.

To contribute a specific property view, it is necessary to implement a PropertyViewDataService which gathers the property data for a selection as well as a PropertyViewWidgetProvider which provides a suitable content widget to display the property data for a specific selection inside the property view widget.

How to test

• Build Theia and open either the browser or electron example.
• Open any workspace (which preferably is holding directories and files, otherwise please add a few)
• Open the property view either via menu View -> Properties or via Shortcut Shift+Alt+P
• Select a file or directory and view its properties in a tree widget structure
• Other selections (or deselections of files/directories) should result in an empty property view (No properties available.)
• Tooltip is available for the property items

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

Example implementation

Here is a short example of how to implement an additional property view, which displays the name and whether it is a file or directory in a simple React widget based on the selection from the file explorer (assuming there would be no ResourcePropertyViewWidget of course):

The FileInfoPropertyDataService gathers the file information and delivers a custom object:

export interface FileInfoPropertyObject {
    name: string;
    isDirectory: boolean;
}
 
@injectable()
export class FileInfoPropertyDataService implements PropertyDataService {
 
    readonly id = 'fileinfo';
    readonly label = 'FileInfoPropertyDataService';
 
    @inject(LabelProvider) protected readonly labelProvider: LabelProvider;
 
    canHandleSelection(selection: Object | undefined): number {
        return this.isFileSelection(selection) ? 1 : 0;
    }
 
    private isFileSelection(selection: Object | undefined): boolean {
        return !!selection && Array.isArray(selection) && FileSelection.is(selection[0]);
    }
 
    async providePropertyData(selection: Object | undefined): Promise<FileInfoPropertyObject | undefined> {
        if (this.isFileSelection(selection) && Array.isArray(selection)) {
            return {
                name: this.labelProvider.getName(selection[0].fileStat.resource),
                isDirectory: (selection[0].fileStat as FileStat).isDirectory
            };
        }
        return Promise.reject();
    }
}

The FileInfoPropertyWidget is a simple ReactWidget and displays the selected node and whether it is a file or directory:

class FileInfoPropertyViewWidget extends ReactWidget implements PropertyViewContentWidget {
 
    static readonly ID = 'file-info-property-view';
    static readonly LABEL = 'File Information';
 
    protected currentFileInfo: FileInfoPropertyObject;
 
    constructor() {
        super();
        this.id = FileInfoPropertyViewWidget.ID;
        this.title.label = FileInfoPropertyViewWidget.LABEL;
        this.title.caption = FileInfoPropertyViewWidget.LABEL;
        this.title.closable = false;
        this.node.tabIndex = 0;
    }
 
    updatePropertyViewContent(propertyDataService?: PropertyDataService, selection?: Object | undefined): void {
        if (propertyDataService) {
            propertyDataService.providePropertyData(selection).then((fileInfo: FileInfoPropertyObject) => this.currentFileInfo = fileInfo);
        }
        this.update();
    }
 
    protected render(): React.ReactNode {
        return <div>
            {`Selected node in explorer: ${this.currentFileInfo.name} ${this.currentFileInfo.isDirectory ? '(Directory)' : '(File)'}`}
        </div>;
    }
}

The FileInfoPropertyViewWidgetProvider is responsible to provide the correct PropertyViewContentWidget based on the selection:

@injectable()
export class FileInfoPropertyViewWidgetProvider extends DefaultPropertyViewWidgetProvider {
 
    readonly id = 'fileinfo';
    readonly label = 'FileInfoPropertyViewWidgetProvider';
 
    private fileInfoWidget: FileInfoPropertyViewWidget;
 
    constructor() {
        super();
        this.fileInfoWidget = new FileInfoPropertyViewWidget();
    }
 
    canHandle(selection: Object | undefined): number {
        return this.isFileSelection(selection) ? 1 : 0;
    }
 
    private isFileSelection(selection: Object | undefined): boolean {
        return !!selection && Array.isArray(selection) && FileSelection.is(selection[0]);
    }
 
    provideWidget(selection: Object | undefined): Promise<FileInfoPropertyViewWidget> {
        return Promise.resolve(this.fileInfoWidget);
    }
 
    updateContentWidget(selection: Object | undefined): void {
        this.getPropertyDataService(selection).then(service => this.fileInfoWidget.updatePropertyViewContent(service, selection));
    }
}

In the frontend module of your application you need to bind the FileInfoPropertyDataService as well as the FileInfoPropertyViewWidgetProvider as follows:

bind(PropertyDataService).to(FileInfoPropertyDataService).inSingletonScope();
bind(PropertyViewWidgetProvider).to(FileInfoPropertyViewWidgetProvider)
.inSingletonScope();

Following these few steps should give the reader an idea on how to implement an own property view, consisting of a specific PropertyViewWidgetProvider and PropertyviewDataService.

Outlook

This PR provides the generic capabilities of a global property view that can be extended with specific additional information contributed by specific Theia extensions.
We plan to use this global property view in the context of graphical modeling tools (e.g. diagram editors). A specific use case is, for instance, the implementation of EMF Ecore support for Theia as well as other applications in the context of EMF.cloud.
In EMF.cloud we also have an extension of the property view support of this PR, that allows to dynamically generate editable property views based on JSON schema defined data. If there is interest in the community to have that as part of Theia, we are happy to open a follow-up PR that builds on this PR.

Please see below for an example of a property view (JSON-schema based in this case) in the Theia Ecore editor:

Signed-off-by: Nina Doschek ndoschek@eclipsesource.com

[vince-fugnitto]

@ndoschek thank you for the contribution, I have not looked nor tested the changes thoroughly yet, but it looks very promising. I did notice a bug however where the properties view is not properly cleared when no editors are opened:

[ndoschek]

Thank you! That's a good point, thanks for noticing! I will look into it tomorrow or Thursday the latest.

[ndoschek]

I noticed that widgets opened via the file navigator are not properly activated, which led to missing global selections. This caused the missing update of the property view.
In the method previewNode the WidgetOpenMode was set to reveal. I removed that part to ensure to use the default mode activate. Now it is possible for all available preference combinations (Editor > Enable Preview as well as Workbench > List > Open Mode > singleClick | doubleClick) to ensure correct global selections if widgets are opened via a node in the file navigator.
Please let me know if you see any problems or side-effects with this solution.

[vince-fugnitto]

The functionality works very well 👍 I believe it is a very good initial contribution to the property-view.

I have some minor cosmetic changes at the moment, please be sure to rebase the pull-request and resolve the conficts.

[vince-fugnitto]

Can you clarify why the change is necessary? I do not believe we should change the behavior of opening nodes in the following pull-request.

[ndoschek]

I took a deeper look at the behaviour of the file/editor selection itself.
Depending on the preference settings, the editor can either be revealed or opened (and therefore automatically activated).
Regarding the revealed mode (which is currently the case for the preferences Editor > Enable Preview > true as well as Workbench > List > Open Mode > singleClick):
If the editor is revealed, the global selection is not updated and does not contain the revealed editor, as the file node in the navigator stays the current selection. (This is also observable in the GIF from your first comment)

Therefore I would say the behaviour is technically correct as the property-view listens to the current selection,
but I agree that it might not be very intuitive to the user.

I can think of two possible ways to go on from here:

• Stick to the behavioural change and activate editors if they are opened via singleClick or in preview to assure that the global selection is up to date.
• Adapt the EditorWidget and EditorPreviewWidget to set the global selection if they are revealed.

What's your opinion on this? Thanks in advance.

[vince-fugnitto]

I took a deeper look at the behaviour of the file/editor selection itself.
Depending on the preference settings, the editor can either be revealed or opened (and therefore automatically activated).
Regarding the revealed mode (which is currently the case for the preferences Editor > Enable Preview > true as well as Workbench > List > Open Mode > singleClick):
If the editor is revealed, the global selection is not updated and does not contain the revealed editor, as the file node in the navigator stays the current selection. (This is also observable in the GIF from your first comment)

Therefore I would say the behaviour is technically correct as the property-view listens to the current selection,
but I agree that it might not be very intuitive to the user.

I can think of two possible ways to go on from here:

• Stick to the behavioural change and activate editors if they are opened via singleClick or in preview to assure that the global selection is up to date.

I do not believe we should modify the current activate behavior.

• Adapt the EditorWidget and EditorPreviewWidget to set the global selection if they are revealed.

This may be the correct behavior, although I'm not sure myself as I have yet to verify if there are any implications when setting the global selection in this case. Would it be possible to rely on the shell to determine the current widget and update the property-view then? (this may also cause issues)

• https://eclipse-theia.github.io/theia/docs/next/classes/core.applicationshell-2.html#onDidChangeCurrentWidget

[ndoschek]

I agree with you and reverted the current activate behaviour.
The combination of selection and focus is the main point to look at in my opinion. As it depends on the settings, how editors are opened, I think sticking to the global selection and focus without relying on changes regarding the ApplicationShell might be the cleanest way for the property-view overall.
Please see my attached screencast, where I show the differences between activated editors and 'only revealed' editor. If editors are revealed, the focus stays in the file navigator widget, until the user decides to activate an editor (or any other widget). This would mean that the property-view would depend only on the global selection/focus behaviour and would align without adaptions if there will be changes in the future on that matter.

[vince-fugnitto]

@ndoschek the changes are very well implemented and work very well, thank you for updating the pull-request 👍 Given that the extension is currently not pulled by any other @theia extension it should be fine to merge and update the property-view functionality iteratively without any risk.

I did have minor comments however if you'd like to take a look:

We may want to think about implementing onActivateRequest for the widget, since currently when it is opened a warning is logged (since the default implementation is no-op). For such a case we can activate the first node in the property-view if you believe it is the proper behavior:

[vince-fugnitto]

@ndoschek thank you for the contribution, I waited for others to review as well but I believe it is fine to merge since it does not affect other extensions. Would you mind resolving the conflict and I'll merge right after?

[ndoschek]

Thank you very much @vince-fugnitto! I rebased my changes to resolve the conflicts.

[paul-marechal]

LGTM, it worked for me by following the steps.

@eclipse-theia/core is anyone ok with taking in this new extension or should it be in its own repo?

I am not against having it here.

[marcdumais-work]

@ndoschek thanks for this contribution. Since SAP Eclipse Source is an Eclipse Foundation member company, I think we do not need to register a CQ for this substantial contribution (> 1000 LoC). Can you please look at the official flowchart(eclipse legal process pdf) and confirm - I think this contribution would fit under the figure 2 case, correct? If so can you also answer the question on figure 7? :

Confirm that the Contribution:
1. Contains No Cryptography
2. Is Developed from Scratch (without
incorporating content from
elsewhere) – Contact the EMO if
you aren’t sure.
3. Is 100% Project License Code

[ndoschek]

@marcdumais-work I looked at the flowchart and can confirm that Figure 2 applies, and can answer both Figure 2 and Figure 7 with yes.

[marcdumais-work]

Let me rephrase Paul's question above:

@eclipse-theia/core is anyone not ok with taking in this new extension, in this repo? If we store it here, it will be published along the other framework extensions, with minimal overhead. If it were in its own repo instead, someone would need to release it from there, separately.

[paul-marechal]

@ndoschek Do think it would be worthwhile to add some UI tests for it? I don't see much that could be unit-tested, but do you think anything could be tested UI-wise to catch potential regressions? (adding to the examples/api-tests test suite).

[ndoschek]

@marechal-p I added the PropertyViewContribution to the example api-tests 'Views' and 'Menus' for now, but I'm not sure if I can think of any valuable addition to the UI tests at the moment.

[paul-marechal]

LGTM, pinging @eclipse-theia/core again because of the new extension.

[marcdumais-work]

@ndoschek: Just a quick note to ask that you please squash this PRs commits before eventual merge. In the meantime it's ok to add new ones on top, while making improvements and answering review comments.

[ndoschek]

@marcdumais-work All commits in this PR are squashed and rebased to latest master.

[JonasHelming]

@marcdumais-work : Was squashing the only remaining change you requested? (that has been done)

[JonasHelming]

@ndoschek Could you resolve the conflicts?

[marcdumais-work]

@ndoschek is the CI failure something to worry about?

[ndoschek]

@marcdumais-work I think the failures were mainly reported because the master branch on my forked repo was not up-to-date and therefore the builds failed, sorry for that!
I disabled the workflow on my fork now, and it looks as the build runs through for this PR.

[marcdumais-work]

@marechal-p if you have no further comments, can you merge this contributions?

[marcdumais-work]

and/or @vince-fugnitto

Commits are squashed.

[vince-fugnitto]

@ndoschek @JonasHelming some minor updated and it should be good to merge (I'll take care of merging after) 👍 Please be sure to squash after making the commits as well.

[ndoschek]

@vince-fugnitto Thank you for your review, I addressed your comments and squashed the commit.

[vince-fugnitto]

@vince-fugnitto Thank you for your review, I addressed your comments and squashed the commit.

@ndoschek looks like something was merged prior to the update which resulted in a conflict. Can you please rebase?

[vince-fugnitto]

Merging! Thank you for your contribution and patience @ndoschek :)