Skip to content

Support multiple decorations in editor glyph margin #179725

@joyceerhl

Description

@joyceerhl

In order to support share link decorations in the glyph margin for my share contribution proposal, I started down the path of reconciling multiple decorations with #179657. I missed an existing issue with discussions #114776, which Connor pointed me to in standup today. I don't think my PR can be merged as is, from standup feedback, there are two main concerns with the approach I took, which does the naive thing of expanding the glyph margin to accommodate decorations:

  1. Anything more than 3 decorations in the glyph margin starts to look unreasonable
  2. The current behavior doesn't render decorations at predictable offsets, which impacts muscle memory e.g. for setting breakpoints (and begs for some kind of 'lane' treatment per decoration type)

Therefore I'm opening this issue to discuss the approach we take, which should address the following issues (I'm creating a new issue to avoid mass-pinging all existing issue subscribers):

Summary of existing discussion

Summarizing some alternative proposals from the above issue thread #114776 and from standup, there are two main questions to solve:

  1. How should we render decorations if there are multiple?
    1. Have dedicated lanes for decoration types
      • We would merge all decorations into a single lane if no line has more than one decoration type, to preserve the nice behavior that test coverage extensions offer
      • This still potentially suffers from pathological cases with lots of decorations so requires some kind of overflow behavior
    2. Have one dedicated lane for breakpoints (common case), and show all other decorations in a second lane
    3. For overflow behavior, sort by each decoration's zIndex to figure out which decoration to render as the placeholder in the overflow
    4. For common cases like testing and debug, the decorations can specify how to merge them to avoid overflow
  2. Many but not all decorations are actionable, how do we decide what action to take?
    1. Have decorations provide an onClick handler, if there's only one decoration registering an onClick handler we should execute that handler, otherwise show the context menu
    2. Show overflowed decorations on hover (since the user still needs to be able to see non actionable decorations) and allow to click on decorations to execute their onClick handlers if any

Proposed approach

I'd like to rework my PR to incorporate the proposals that the team has suggested. Here's what I'm thinking of doing, please let me know what you think:

  1. Create at most two lanes in the glyph margin
    • One lane is reserved for breakpoints (the hint as well as the actual breakpoint/logpoint)
    • Another lane is reserved for all other decorations, actionable or otherwise
      • This includes the diff revert icon, testing decorations, and extension-contributed decorations
      • If there is more than one decoration to render, we will decide which one to show based on the decoration's specified zIndex
        • Any lower priority decorations will be shown in a hover that still allows users to select or click the decorations, this is similar to what Atom did
        • It should be possible to tab to the overflowing decoration and hit tab to trigger the hover, then use left and right arrows to navigate between decorations. There should also be descriptive alt text for the decorations whether or not they are connected to onClick handlers
    • One question is whether to create space for one or two lanes if there are no decorations other than breakpoints to be rendered. I propose that we should create two lanes in order to avoid the glyph margin resizing in order to create the lane for a breakpoint hover hint if the user hovers over a non-breakpoint decoration and wants to set a breakpoint (though if this ends up behaving poorly I'm also happy to just have one lane)
  2. Add support for optional onClick handlers in decorations and adopt them in core-contributed decorations
    • This is to support interacting with overflowed decorations in the proposed hover runs associated actions, which aligns with user expectations and achieves parity with what breakpoints have (today breakpoints and testing have separate context menu handlers)
      • Breakpoint clicks will work the way they currently do
      • If there are overflowing decorations and only the highest-priority decoration has a click handler, we will run that decoration's click handler when the user clicks that decoration
      • If there are overflowing decorations and a decoration which is not the highest-priority decoration has a click handler, we will display the hover
    • Expose support for running extension-contributed actions in decorations
      • This could be a command attribute in TextEditorDecorationType, or an event vscode.window.onDidSelectTextEditorDecoration

cc @alexdima

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions