You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
When the dropdown covers the constructor call, the arrow should be displayed under the constructor name.
When the dropdown covers the argument placeholder without value, the arrow should be centered under the argument name.
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
Allow multiple widget definitions with the same implementation, but different priorities and/or matchers.
Allow multiple priorities for a single widget.
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:
We try to match widgets normally.
If one is selected, we look if it wants to be wrapped by WidgetArgumentName
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:
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.
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).
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.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Date: 2024-05-07
Participants: @vitvakatu, @Frizi, @farmaazon
Topic: Intricacies of the
WidgetSelectionArrow
implementationDescription 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:
Because of how the argument name display is working right now, last two cases pose significant problem.
We need to place the
WidgetSelectionArrow
either:or:
I.e., we need to either wrap
WidgetArgumentName
or be wrapped by it. Moreover, our final decision depends on theWidgetArgumentName
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
It was almost immediately discarded because of probable code duplication across widget variants.
2. Split
WidgetArgumentName
intoWidgetArgumentName
andWidgetPlaceholder
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
(orwrapByWidget = [ArgumentNameWidget]
), and thus controls the widget selection order with regards of the wrapper widget. The algorithm becomes as follows:WidgetArgumentName
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:
IwantToBeWrappedByArgumentName
WidgetArgumentName
)This implementation will help to solve two different issues with our widgets:
WidgetSelection
to ensure proper ordering with respect toWidgetArgument
. Instead, we would simply state thatWidgetSelection
never wants to be wrapped inWidgetArgumentName
.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, theWidgetSelection
will draw it by itself.In this case,
WidgetSelectionArrow
will have lower priority than theWidgetArgumentName
, and will not handle case (2).This solution was selected as final, with a small improvement. Instead of duplicating
drawArrow()
betweenWidgetSelection
andWidgetSelectionArrow
, the former canTeleport
arrow HTML to theWidgetSelectionArrow
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.
Beta Was this translation helpful? Give feedback.
All reactions