-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Nav toolbar - impements #9418 #31162
Conversation
@mihailik, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @jrieken to be potential reviewers. |
Updated screenshot. |
It would be nice to see it on nightly. |
This is pretty cool and something I have been missing. Indeed, my office was opposite of the guy that did it for Eclipse. You are on the right path but there are some bits missing:
Dude, this is pretty cool and I am existing to having this, tho it is still a bit of work |
I did fork the API work to #34968 |
Thanks @jrieken -- agree on all points! In fact I tried to handle tree/symbol/range structure on a local branch, but turned into a rabbit hole, ground it to a halt. Cleaner API + some feedback = I am excited to push ahead! I assume while waiting for the #34968 API update, I should rearrange the code/styling. If you give me some clues, point towards existing code to copy from I'll set out to work on that. |
It does need some input from @alexandrudima to support contributing a widget to the editor that pushes down all content, like like a |
To keep things clean, it would be quite important that the logic is moved out of To achieve this, there needs to be a new
Now, this contribution needs new APIs from Then, it would be the responsibility of |
@alexandrudima — I want to simplify the widget-side design:
The trouble with more generic approach, it affects too many places and would be too risky a PR to pass. The more generic Higher concern is that allowing decorative crust on all 4 edges could lead away from uncluttered design objective of VSCode. With simplified design, the new And |
Here is what I had in mind: const enum EdgeWidgetPosition { N, E, S, W }
interface IEdgeWidget {
getId(): string;
getDomNode(): HTMLElement;
/** returns the location where the widget would like to sit */
getPosition(): EdgeWidgetPosition;
/** returns the width (for E or W widgets) or the height (for N or S widgets) in px */
getSize(): number;
}
interface ICodeEditor {
...
/** will call getPosition() and getSize() once to layout the widget and will insert getDomNode() to the DOM. */
addEdgeWidget(widget: IEdgeWidget): void;
/** will call getPosition() and getSize() once to layout the widget. */
layoutEdgeWidget(widget: IEdgeWidget): void;
/** will remove the dom node from the DOM */
removeEdgeWidget(widget: IEdgeWidget): void;
} Implementation wise, on the code editor side, a separate object can be allocated per contributed widget: interface IEdgeWidgetData {
domNode: HTMLElement;
position: EdgeWidgetPosition;
size: number;
source: IEdgeWidget;
}
class CodeEditorWidget implements ICodeEditor {
...
private _edgeWidgets: IEdgeWidgetData[];
...
// - to add an edge widget, a new `IEdgeWidgetData` is pushed onto `_edgeWidgets` and an editor layout is triggered (getPosition() and getSize() are called once on the widget), the dom node is fetched via getDomNode() and stored in `domNode` and added to the DOM.
// - to layout an edge widget, the widget is located in `_edgeWidgets` and an editor layout is triggered (getPosition() and getSize() are called once on the widget)
// - to remove an edge widget, the `domNode` is removed from the DOM.
...
// layouting needs to loop over contributed edge widgets, in the order they were contributed
// and split them in the 4 cases (N, E, S, W)
// Then, the dom node are absolutely positioned and positions are computed
// Finally, the remaining width and height is the one that is somehow passed into the `Configuration` object. This might require a bit of refactorings to work smoothly.
} |
5f0224b
to
85220d2
Compare
Starting anew, going with There are two similar things: Can't see what was meant by 'contribution' in here, so will press ahead with approach closely following P.S. The old branch is preserved as nav-toolbar-save |
@alexandrudima drudima what I'm really struggling to figure is the lifecycle and ownership of EditorLayoutInfo. When edge widget is added (say a toolbar) it must squeeze the height of the editor content area. Stepping through the code it goes in EditLayoutProvider.compute. It digests sizes, metrics, constraints and allocates editor-specific sizes, including Does it make sense, am I on the right track? |
…ntLeft,contentWidth,contentHeight).
…ntTop in options.compute.
Fully functional, but not very well styled navigational toolbar, as described in #9418 Feature Request : Navigation Bar like Visual Studio or Eclipse
Clicking on the breadcrumb element scrolls editor and places cursor onto it. Also flashes the symbol momentarily. Neat navigation!
On startup it takes a little while to appear (language service need to initialise). Then it follows cursor movement and editing of course!
By relying on standard
DocumentSymbolProvider
, it just works with whatever lexical model current editor uses. Picks up deep parsing features in TypeScript, and uses dumber match editing JSON.Missing pieces
Styling is beyond naive: just applying CSS within code. I'm keen to do it properly, but need guidance please!
Diff mode only populates navbar for edited panel, the original code isn't decorated with navbar. It looks silly on screen, and could be helpful to enable in Diff/original too.
Everything is in one file -- should I separate the logic from that
codeEditor
somewhere else? Any guidance is welcome!No tests please guide me what's the best approach here: where should tests go?
More features to add