Skip to content
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

Test: glyph margin widgets #186210

Closed
3 tasks done
joyceerhl opened this issue Jun 26, 2023 · 2 comments
Closed
3 tasks done

Test: glyph margin widgets #186210

joyceerhl opened this issue Jun 26, 2023 · 2 comments

Comments

@joyceerhl
Copy link
Contributor

joyceerhl commented Jun 26, 2023

Refs: #176316

Authors: @joyceerhl @alexdima
Complexity: 4

Create Issue


We've added support for GlyphMarginWidgets in the code editor. This allows decoration authors in core only (not extension authors) to handle clicks on glyph margin decorations by passing in a DOM node that has attached listeners.

Test: please take a look at the IGlyphMarginWidget interface and the new methods for adding, layouting, and removing widgets. Verify that the methods and properties make sense, are properly documented, and behave as expected.

edit from hediet: Try out the new API in the monaco editor playground!

Refs:

  • /**
    * A glyph margin widget renders in the editor glyph margin.
    */
    export interface IGlyphMarginWidget {
    /**
    * Get a unique identifier of the glyph widget.
    */
    getId(): string;
    /**
    * Get the dom node of the glyph widget.
    */
    getDomNode(): HTMLElement;
    /**
    * Get the placement of the glyph widget.
    */
    getPosition(): IGlyphMarginWidgetPosition;
    }
  • /**
    * Add a glyph margin widget. Widgets must have unique ids, otherwise they will be overwritten.
    */
    addGlyphMarginWidget(widget: IGlyphMarginWidget): void;
    /**
    * Layout/Reposition a glyph margin widget. This is a ping to the editor to call widget.getPosition()
    * and update appropriately.
    */
    layoutGlyphMarginWidget(widget: IGlyphMarginWidget): void;
    /**
    * Remove a glyph margin widget.
    */
    removeGlyphMarginWidget(widget: IGlyphMarginWidget): void;
@joyceerhl joyceerhl added this to the June 2023 milestone Jun 26, 2023
@hediet
Copy link
Member

hediet commented Jun 26, 2023

This looks like an ideal use case to try the monaco editor playground!
https://microsoft.github.io/monaco-editor/playground.html?source=v0.40.0-dev.20230626#example-creating-the-editor-hello-world

@hediet
Copy link
Member

hediet commented Jun 27, 2023

Works nicely!

Just some notes/thoughts:

  • Why does a widget have a range and not just a position? Can you add some docs there?
  • You followed the same API that Content and Overlay widgets have, but I think that API is quite outdated (addGlyphMarginWidget should return a disposable instead of having removeGlyphMarginWidget, layoutGlyphMarginWidget could be replaced by an "onChange" event and I don't think getId is needed).
    • Personally, I'd prefer a more modern API over consistency for the monaco editor (and would eventually refactor the other APIs as well), but I can understand the other side as well
  • Just curious, for what feature did you add GlyphMarginWidgets?

@hediet hediet removed their assignment Jun 27, 2023
@roblourens roblourens removed their assignment Jun 27, 2023
@connor4312 connor4312 removed their assignment Jun 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants