Implement Dropdown Widgets. #8411
Conversation
MichaelMauderer
added
CI: No changelog needed
| <div class="WidgetDrodown"> | ||
| <DropdownWidget | ||
| v-if="showDropdownWidget" | ||
| :color="parentColor" |
There was a problem hiding this comment.
I think this could just be a constant color="var(--node-color-primary)"
There was a problem hiding this comment.
Actually, it seems that var(--node-color-primary) can only be used in CSS. But this got me thinking about a workaround: get the style variable from an HTML element. This seems to work as intended now.
There was a problem hiding this comment.
the color prop is passed directly to CSS though, so i believe @Frizi's suggestion should theoretically work
| <DropdownWidget | ||
| v-if="showDropdownWidget" | ||
| :color="parentColor" | ||
| :values="tagLabels ?? []" |
There was a problem hiding this comment.
This ?? looks unnecessary, I think tagLabels should always be at least an empty array.
| <span class="WidgetArgumentName" @pointerdown="showDropdownWidget = !showDropdownWidget"> | ||
| <template v-if="showArgumentValue"> | ||
| <span class="name">{{ argName }}</span | ||
| ><span> {{ selectedValue }} </span> | ||
| </template> | ||
| <template v-else>{{ argName }}</template> | ||
| </span> |
There was a problem hiding this comment.
this part of the template is incorrect, you shouldn't be handling argument names within the drodpowns explicitly. We still want to show more complicated widget trees on elements that have dropdown choices, e.g to show nested function calls or array widgets within the selected expression. To properly render the dropdown as a wrapper widget around other widgets, you should use <NodeWidget :input="props.input" /> in some place. That way you allow other matching widgets to attach to the same node.
| </template> | ||
| <template v-else>{{ argName }}</template> | ||
| </span> | ||
| <div class="WidgetDrodown"> |
There was a problem hiding this comment.
Inconsistent class. It should be the same as the component name. Also, how about naming it WidgetSingleChoice?
| // When the selected index changes, we update the expression content. | ||
| watch(selectedIndex, (_index) => { | ||
| console.warn('TODO: Update the expression content once the AST is updated.') | ||
| const id = '' // TODO: Get the id of the expression once the AST is updated. |
There was a problem hiding this comment.
To make it work at least in case of existing argument, you could use
const id = props.input instanceof ArgumentAst ? props.input.ast.astId : undefined
| @@ -34,7 +34,7 @@ export const widgetDefinition = defineWidget([ArgumentPlaceholder, ArgumentAst], | |||
| <template> | |||
| <span class="WidgetArgumentName" :class="{ placeholder, primary }"> | |||
| <template v-if="showArgumentValue"> | |||
| <span class="name">{{ props.input.info!.name }}</span | |||
| <span class="value">{{ props.input.info!.name }}</span | |||
There was a problem hiding this comment.
Not sure why is this changed? That span is actually representing the argument name, not its value. The "value" is represented by the NodeWidget below.
| <template> | ||
| <div ref="rootElement" class="WidgetRoot"> | ||
| <span class="WidgetArgumentName" @pointerdown="showDropdownWidget = !showDropdownWidget"> | ||
| <template v-if="showArgumentValue"> |
There was a problem hiding this comment.
This ref is always true, right? I think you could remove this branch completely.
| <div ref="rootElement" class="WidgetRoot"> | ||
| <span class="WidgetArgumentName" @pointerdown="showDropdownWidget = !showDropdownWidget"> | ||
| <template v-if="showArgumentValue"> | ||
| <NodeWidget :input="props.input" /><span class="value"> {{ selectedValue }} </span> |
There was a problem hiding this comment.
I'm not super sure why you need to display the selectedValue here, since the NodeWidget should on its own represent that value properly. I believe this may lead to repeated text.
Pull Request Description
Closes #8259
Peek.2023-11-28.12-22.webm
Important Notes
Some TODOs left for after the AST refactoring:
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.