WidgetSelectionArrow implementation

[vitvakatu]

Date: 2024-05-07
Participants: @vitvakatu, @Frizi, @farmaazon
Topic: Intricacies of the WidgetSelectionArrow implementation

Description of the problem

As stated in #9428, we want to display arrow of the WidgetSelection (the dropdown) directly below the constructor name, not centered under the whole property access chain. As a broader task, we want to control the displayed position of the arrow so that it is not always aligned to the dropdown itself (which can take quite some place on the node), but rather to specific AST or placeholder inside the dropdown.

There are a few distinct cases here:

1. When the dropdown covers the constructor call, the arrow should be displayed under the constructor name.
2. When the dropdown covers the argument placeholder without value, the arrow should be centered under the argument name.
3. When the dropdown covers the argument placeholder with value or the actual argument with the argument name, the arrow should be centered under the value.

Because of how the argument name display is working right now, last two cases pose significant problem.

We need to place the WidgetSelectionArrow either:

WidgetSelection
> Some widgets...
> WidgetSelectionArrow
> > WidgetArgumentName
> > > Some widgets...

or:

WidgetSelection
> Some widgets...
> WidgetArgumentName
> > WidgetSelectionArrow
> > > Some widgets...

I.e., we need to either wrap WidgetArgumentName or be wrapped by it. Moreover, our final decision depends on the WidgetArgumentName implementation – whether it displays some value or not. More specifically, it depends on the instantiation of other widgets down the widget tree.

Our widget system does not support such things, as priorities of widgets are completely static, and the widget can’t decide whether it wants to be considered before WidgetArgumentName or after it dynamically.

Proposed solutions

1. Modify widget selection to allow multiple tries on a single widget

1. Allow multiple widget definitions with the same implementation, but different priorities and/or matchers.
2. Allow multiple priorities for a single widget.
3. Define two separate arrow widgets.

It was almost immediately discarded because of probable code duplication across widget variants.

2. Split WidgetArgumentName into WidgetArgumentName and WidgetPlaceholder

Ultimately solution (1), but reversed. Same reasons to discard.

3. Make the argument name a special property of the input

Basically, make WidgetArgumentName a very special part of the widget tree, embedding it into any other widget. Each widget can request the argument name to be displayed or not. Remove WidgetArgumentName as a separate widget, and thus solve the issue.

Was eventually discarded because of unclear implementation and lack of consistency (yes, many widgets want argument names, but definitely not all of them).

4. Allow widgets to decide whether they want to be wrapped by the WidgetArgumentName

We discussed several similarly looking implementations when the widget definition basically has a property wrapByArgumentName = true (or wrapByWidget = [ArgumentNameWidget]), and thus controls the widget selection order with regards of the wrapper widget. The algorithm becomes as follows:

1. We try to match widgets normally.
2. If one is selected, we look if it wants to be wrapped by WidgetArgumentName
3. If it does, instantiate WidgetArgumentName, then continue normally.

WidgetArgumentName will have the lowest priority (allowing empty placeholders), but as most widgets want to be wrapped by argument name, it will almost always be selected up the tree.

There are different possible implementations here, including:

• having a single boolean IwantToBeWrappedByArgumentName
• having a widget reference or name specified (allowing arbitrary wrappers, not only WidgetArgumentName)
• having a list of the above

This implementation will help to solve two different issues with our widgets:

1. Case when we need to specifically check against dynamic config in WidgetSelection to ensure proper ordering with respect to WidgetArgument. Instead, we would simply state that WidgetSelection never wants to be wrapped in WidgetArgumentName.
2. Separate implementation of argument name display in WidgetCheckbox, because we always want to display argument name for checkboxes, even if it is not a top-level argument.

However, after a closer look, this implementation does not solve our original problem – we still can’t decide whether the arrow wants to be wrapped by argument name or not.

We decided to keep track of this solution as potentially interesting.

5. Propagate information about (not) selected arrow up the tree

Using Contexts, we can propagate information about the arrow widget being instantiated somewhere down the tree – either by assigning some ref or by using a callback. If the arrow is not handled by the WidgetSelectionArrow somewhere down the tree, the WidgetSelection will draw it by itself.

In this case, WidgetSelectionArrow will have lower priority than the WidgetArgumentName, and will not handle case (2).

// WidgetSelection
const arrowInstantiatedDownTheTree = ref()
provideContext(arrowInstantiatedDownTheTree)
// ...
if (!arrowInstantiatedDownTheTree)
	drawArrow()

// WidgetArrow
const context = injectContext()
context.arrowInstantiatedDownTheTree = true
drawArrow()

This solution was selected as final, with a small improvement. Instead of duplicating drawArrow() between WidgetSelection and WidgetSelectionArrow, the former can Teleport arrow HTML to the WidgetSelectionArrow if requested through the context.

Separate discussions

Shouldn’t we remove Score::Good?

Not, it is still helpful in certain situations, mostly when considering dynamic config. When the dynamic config is present and is favorable, we return Score::Perfect; otherwise, Score::Good. As an example, if a certain method accepts numbers, but has dynamic config for a vector, we want to display the vector literal.

Similarly, should we use more specific Selected::ByDynamicConfig, Selected::ByTypeInfo, Selected::ByAST, etc?

We can consider this, but there are no strong arguments for this change.