Add breadcrumbs bar to editor widget #6371

corneliusludmann · 2019-10-11T15:10:17Z

What it does

This commit adds a breadcrumbs bar to the editor widget. It shows the path to the current file and outline information as breadcrumbs. A click of breadcrumbs allows to switch to siblings.

This fixes #5475.

How to test

Open an editor shows the breadcrumbs on top of the editor. Folders with files are clickable. A popup allows to select a file that will be opened immediately. Outline information are added to the breadcrumbs as well.

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

AlexTugarev · 2019-10-11T15:13:56Z

@corneliusludmann please make sure you've signed an ECA https://accounts.eclipse.org/user/eca

corneliusludmann · 2019-10-11T15:15:41Z

@corneliusludmann please make sure you've signed an ECA https://accounts.eclipse.org/user/eca

Currently working on it … ::-)

vitaliy-guliy · 2019-10-12T20:49:26Z

@corneliusludmann could you attach few screenshots?

corneliusludmann · 2019-10-12T23:30:25Z

@corneliusludmann could you attach few screenshots?

Sure. This GIF shows the breadcrumbs in action:

corneliusludmann · 2019-10-12T23:41:46Z

Attention: I came across with an issue that occurs when opening the Preferences widget after this commit as described here: #6377. I proposed an solution there: #6378. I suggest fixing that issue first and then rebase this PR.

packages/editor/package.json

akosyakov · 2019-10-13T04:03:52Z

How can one disable them? Can we have breadcrumbs.enabled as in VS Code?

akosyakov · 2019-10-13T04:06:13Z

It does not work like in VS Code: when one click on the part of a path VS Code allows to siblings files, not to chlid as in this PR.
for editor-widget.ts editor, editor segment is not clickable at all
when something opens it only allows to open files, not folders

packages/editor/src/browser/breadcrumbs/breadcrumbs.css

packages/editor/src/browser/breadcrumbs/breadcrumbs-utils.ts

akosyakov · 2019-10-13T04:12:58Z

packages/monaco/src/browser/monaco-outline-contribution.ts

-        });
+
+        // updateOutline and handleCurrentEditorChanged need to be called even when the outline view widget is closed
+        // in order to udpate breadcrumbs.


i don't understand it, if an editor is closed there should not be breadcrumbs.

The removed condition does not check if an editor is open or closed it checks if the outline view is open or closed. Previously, the outline changes are propagated when the outline view is open only. Now we need the information even when the outline view is closed because the outline view is not the only one that uses this information anymore. The breadcrumbs need the information as well.

What we could do is to check whether the outline view is open OR the breadcrumbs are visible (by the preferences entry breadcrumbs.enabled). I don't know if it is really worth it.

akosyakov · 2019-10-13T04:15:28Z

packages/editor/src/browser/editor-frontend-module.ts

+    bind(BreadcrumbsRendererFactory).toFactory(ctx =>
+        (editor: TextEditor) => {
+            const childContainer = ctx.container.createChild();
+            childContainer.bind('TextEditor').toConstantValue(editor);


please use a symbol, if there are duplicate extensions installed, they can pick a bogus instance by a string

akosyakov · 2019-10-13T04:26:40Z

Why don't we reuse file and outline tree widgets to open URIs? We have already them, have to create new instances and render on click?

I would also prefer if we have something like BreadcrumbService and BreadcrumbContribution in core which are URI and DOM event based, so:

editor extension is using it to open a URI from breadcrumbs, i.e. BreadcrumbService.open(uri, {x, y})
filesystem extension implements BreadcrumbContribution to open a new file tree widget for the given
URI
workspace extension overrides FileSystemBreadcrumbContribution in order to customize some bits
outline implement BreadcrumbContribution to open a new outline tree widget for the given URI if it has ... as a fragment

lmcbout · 2019-10-15T15:48:30Z

Note: using Theia in the workspace
Close all editors, then open package.json

Found some issues when playing with this PR:

1- No folders available from the breadcrumbs pull down
2- First click in the editor does not expand the breadcrumbs menu, need to click on another line to expand it
3-  In VSCode, there is "..." appearing at the end when opening the file when there is no selection
    In VSCode: When selecting at the end of the line for "name" , three dots shows for the selection, 
    In Theia, navigating to the Name  keeps the last structure selection, i.e. engines | devDependencies 
    when selecting after the last char of the line, no refresh.

VSCode:

Theia:

corneliusludmann · 2019-10-15T16:23:16Z

Found some issues when playing with this PR:

Thank you for your helpful feedback.

1- No folders available from the breadcrumbs pull down

Yes, that is a limitation I forgot to mention in the PR description. I currently working on adding the existing file tree view in the popup. I had it planned as the next evolutionary step. Sorry I didn't mention it.

2- First click in the editor does not expand the breadcrumbs menu, need to click on another line to expand it

That seems to be a bug in the existing outline view implementation. When you carefully observe the outline view you will see that on first click the outline is rendered without highlighting the current selection. Only after the second click the selection is highlighted. Thus, fixing this will fix this issue as well. I gonna file an issue for that if not already exist.

3- In VSCode, there is "..." appearing at the end when opening the file when there is no selection

Seems to be another limitation I was not aware of. Consider to implement this as well.

In VSCode: When selecting at the end of the line for "name" , three dots shows for the selection,
In Theia, navigating to the Name keeps the last structure selection, i.e. engines | devDependencies
when selecting after the last char of the line, no refresh.

Oh, yes, you're right. Outline breadcrumbs are not removed when there is no selection. Gonna fix this.

lmcbout · 2019-10-15T17:44:27Z

2- First click in the editor does not expand the breadcrumbs menu, need to click on another line to expand it

That seems to be a bug in the existing outline view implementation. When you carefully observe the outline view you will see that on first click the outline is rendered without highlighting the current selection. Only after the second click the selection is highlighted. Thus, fixing this will fix this issue as well. I gonna file an issue for that if not already exist.

There might be a bug in the outline view implementation, but I think it is more than that, If you look at the following video, After opening the file, I selected "@types/chai" On the first click, we can see the blue bar on top of the editor and the data being displayed in the outline view, but keeping to select the same item over and over does not amend the breadcrumbs until I select another item or double click on the current selection.

corneliusludmann · 2019-10-19T10:26:22Z

I would also prefer if we have something like BreadcrumbService and BreadcrumbContribution in core which are URI and DOM event based, so:

editor extension is using it to open a URI from breadcrumbs, i.e. BreadcrumbService.open(uri, {x, y})

filesystem extension implements BreadcrumbContribution to open a new file tree widget for the given
URI

workspace extension overrides FileSystemBreadcrumbContribution in order to customize some bits

outline implement BreadcrumbContribution to open a new outline tree widget for the given URI if it has ... as a fragment

I refactored the code with commit 03eb644 which introduces BreadcrumbService and BreadcrumbContribution. Fell free to make your comments.

I will squash the commits once I implemented all suggested changes.

corneliusludmann · 2019-10-19T10:29:14Z

How can one disable them? Can we have breadcrumbs.enabled as in VS Code?

Done with commit 3dee5ec.

I will squash the commits once I implemented all suggested changes.

corneliusludmann · 2019-10-19T10:38:34Z

Why don't we reuse file and outline tree widgets to open URIs? We have already them, have to create new instances and render on click?

For the file tree widget, I currently working on it. For the outline tree widget it seems not possible yet because the tree is not easily reusable yet.

corneliusludmann · 2019-10-19T10:40:26Z

In VSCode: When selecting at the end of the line for "name" , three dots shows for the selection,
In Theia, navigating to the Name keeps the last structure selection, i.e. engines | devDependencies
when selecting after the last char of the line, no refresh.

Oh, yes, you're right. Outline breadcrumbs are not removed when there is no selection. Gonna fix this.

Should be fixed with commit 03eb644.

corneliusludmann · 2019-10-19T11:09:16Z

2- First click in the editor does not expand the breadcrumbs menu, need to click on another line to expand it

That seems to be a bug in the existing outline view implementation. When you carefully observe the outline view you will see that on first click the outline is rendered without highlighting the current selection. Only after the second click the selection is highlighted. Thus, fixing this will fix this issue as well. I gonna file an issue for that if not already exist.

There might be a bug in the outline view implementation, but I think it is more than that, If you look at the following video, After opening the file, I selected "@types/chai" On the first click, we can see the blue bar on top of the editor and the data being displayed in the outline view, but keeping to select the same item over and over does not amend the breadcrumbs until I select another item or double click on the current selection.

Hmm. At first, when I look at your video, the moment when the breadcrumbs appear is in sync with the highlighting of the tree item in the outline view. Therefore, I still think it is an general issue with the outline view. Second, when I try to reproduce your steps I realized that it does not have to do with a single or double click, it is just a delay. When I open an editor, click once, wait a few seconds and do a second click the breadcrumbs appear immediately (same with the highlighting of the outline tree item). Sometimes it needs more than a few seconds. However, in my tests, it is always in sync with the outline view highlighting.

akosyakov

I've started reviewing code, but then stopped after realizing that refactoring to reuse trees is not finished yet.

akosyakov · 2019-10-21T04:52:30Z

packages/core/src/browser/breadcrumbs/breadcrumbs-service.ts

+     * Subscribe to this event emitter to be notifed when the breadcrumbs have changed.
+     * The URI is the URI of the editor the breadcrumbs have changed for.
+     */
+    get onBreadcrumbsChange(): Event<URI> {


onDidChangeBreadcrumbs or onBreadcrumbsChanged

akosyakov · 2019-10-21T04:52:56Z

packages/core/src/browser/breadcrumbs/breadcrumbs-service.ts

+     * Notifies that the breadcrumbs for the given URI have changed and should be re-rendered.
+     * This fires an `onBreadcrumsChange` event.
+     */
+    breadcrumbsChanges(uri: URI): void {


firing events should be an implementation detail, i.e. BreadcrumbsContribution should have onDidChange instead

Maybe I don't get it. But it sounds that this would make it more complicated without any substantial benefit since the breadcrumbs renderer has to subscribe to all breadcrumb contributions then.

akosyakov · 2019-10-21T04:53:53Z

packages/core/src/browser/breadcrumbs/breadcrumbs-contribution.ts

+    /**
+     * The breadcrumb type. Breadcrumbs returned by `#computeBreadcrumbs(uri)` should have this as `Breadcrumb#type`.
+     */
+    type: symbol;


readonly ? the same about priority

akosyakov · 2019-10-21T04:54:21Z

packages/core/src/browser/breadcrumbs/breadcrumbs-service.ts

+    /**
+     * Opens a popup for the given breadcrumb at the given position. `parent` is used as the host element for the newly created popup element.
+     */
+    async openPopup(breadcrumb: Breadcrumb, position: { x: number, y: number }, parent: HTMLElement): Promise<BreadcrumbPopup | undefined> {


just open?

akosyakov · 2019-10-21T04:56:46Z

packages/core/src/browser/breadcrumbs/breadcrumb-popup-container-renderer.tsx

+import { BreadcrumbPopup } from './breadcrumb-popup';
+
+export const BreadcrumbPopupContainerRenderer = Symbol('BreadcrumbPopupContainerRenderer');
+export interface BreadcrumbPopupContainerRenderer {


Do we need interface, symbol and a class? Should not BreadcrumbPopupContainerRenderer be enough? Please see https://github.com/eclipse-theia/theia/wiki/Coding-Guidelines#classes-over-interfaces

This commit adds a breadcrumbs bar to the editor widget. It shows the path to the current file and outline information as breadcrumbs. A click of breadcrumbs allows to jump to other files or to code sections. Fixes eclipse-theia#5475 Signed-off-by: Cornelius A. Ludmann <[email protected]>

corneliusludmann · 2019-11-12T11:21:09Z

I updated this PR with the file tree for file breadcrumbs and a '…' breadcrumb for outline breadcrumbs. There are still some minor flaws with the outline breadcrumbs that are in sync with flaws in the outline itself. These need to be fixed in the outline implementation. (E.g. selecting a non-collapsed outline element does not collapse the tree in the outline view and does not set the correct breadcrumb.)

From my point of view, this PR is ready for another review.

akosyakov · 2019-12-09T14:59:52Z

We should register and use breadcrumb colors to be aligned with VS Code: https://code.visualstudio.com/api/references/theme-color#breadcrumbs

lmcbout · 2019-12-09T16:00:18Z

I was looking to get a second review, But there are merge conflicts :(

akosyakov · 2019-12-10T16:05:30Z

Breadcrumb should be contributed to the toolbar of a tab bar group and be updated based on the current widget. It should be enabled for any navigatable widget, at least file parts.

paulalexwilson · 2020-03-10T13:08:04Z

I was excited by this PR. Do we plan to merge it at some point or is it no-longer being considered?

lmcbout · 2020-03-10T13:59:19Z

I am interested by this PR as well, but there was some unresolved and merging issues , they are not resolved yet

corneliusludmann · 2020-03-11T13:18:14Z

I could resolve the merge conflicts and rebase the branch. However @akosyakov raised two requests that need to be implemented before merging:

Currently I do not have the time to take care of it. If someone else is willing to contribute to this work feel free to push to this branch or to fork this branch. I'm happy to provide assistance when needed.

colin-grant-work · 2021-07-02T21:51:25Z

@corneliusludmann, my colleague @kenneth-marut-work and I were thinking of finishing this PR up, unless you were planning on returning to it?

corneliusludmann · 2021-07-04T12:04:21Z

@colin-grant-work, unfortunately, I don't. I would be very happy if you would take over the work on this feature.

AlexTugarev requested review from akosyakov, lmcbout and svenefftinge October 11, 2019 15:11

corneliusludmann force-pushed the GH-5475 branch from 088c615 to 7b17c61 Compare October 12, 2019 23:41

akosyakov reviewed Oct 13, 2019

View reviewed changes

packages/editor/package.json Outdated Show resolved Hide resolved

akosyakov requested a review from AlexTugarev October 13, 2019 04:02

akosyakov added the editor issues related to the editor label Oct 13, 2019

akosyakov reviewed Oct 13, 2019

View reviewed changes

packages/editor/src/browser/breadcrumbs/breadcrumbs.css Outdated Show resolved Hide resolved

akosyakov reviewed Oct 13, 2019

View reviewed changes

packages/editor/src/browser/breadcrumbs/breadcrumbs-utils.ts Outdated Show resolved Hide resolved

akosyakov reviewed Oct 13, 2019

View reviewed changes

akosyakov added filesystem issues related to the filesystem outline issues related to the outline labels Oct 13, 2019

corneliusludmann force-pushed the GH-5475 branch from 7b17c61 to 3dee5ec Compare October 19, 2019 10:20

akosyakov reviewed Oct 21, 2019

View reviewed changes

corneliusludmann force-pushed the GH-5475 branch 2 times, most recently from ab46c44 to 4f4879f Compare November 12, 2019 10:51

corneliusludmann force-pushed the GH-5475 branch from 4f4879f to 509f04e Compare November 12, 2019 11:09

corneliusludmann requested a review from akosyakov November 12, 2019 12:50

jbicker mentioned this pull request Dec 9, 2019

[WIP] VS Code missing colors #6722

Merged

77 tasks

colin-grant-work assigned colin-grant-work and kenneth-marut-work Jul 2, 2021

colin-grant-work mentioned this pull request Aug 17, 2021

Add breadcrumbs for editors #9920

Merged

1 task

colin-grant-work closed this in #9920 Sep 1, 2021

Add breadcrumbs bar to editor widget #6371

Add breadcrumbs bar to editor widget #6371

Conversation

corneliusludmann commented Oct 11, 2019

What it does

How to test

Review checklist

Reminder for reviewers

AlexTugarev commented Oct 11, 2019

corneliusludmann commented Oct 11, 2019

vitaliy-guliy commented Oct 12, 2019

corneliusludmann commented Oct 12, 2019

corneliusludmann commented Oct 12, 2019

akosyakov commented Oct 13, 2019

akosyakov commented Oct 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akosyakov Oct 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akosyakov commented Oct 13, 2019 • edited Loading

lmcbout commented Oct 15, 2019

corneliusludmann commented Oct 15, 2019

lmcbout commented Oct 15, 2019

corneliusludmann commented Oct 19, 2019

corneliusludmann commented Oct 19, 2019

corneliusludmann commented Oct 19, 2019

corneliusludmann commented Oct 19, 2019

corneliusludmann commented Oct 19, 2019

akosyakov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corneliusludmann commented Nov 12, 2019

akosyakov commented Dec 9, 2019

lmcbout commented Dec 9, 2019

akosyakov commented Dec 10, 2019 • edited Loading

paulalexwilson commented Mar 10, 2020

lmcbout commented Mar 10, 2020

corneliusludmann commented Mar 11, 2020

colin-grant-work commented Jul 2, 2021

corneliusludmann commented Jul 4, 2021

akosyakov commented Oct 13, 2019 •

edited

Loading

akosyakov Oct 13, 2019 •

edited

Loading

akosyakov commented Oct 13, 2019 •

edited

Loading

akosyakov commented Dec 10, 2019 •

edited

Loading