You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/09/01 09:26:24 UTC
[GitHub] [superset] kgabryje commented on a change in pull request #16482: fix: ensure array return value in DND selector
kgabryje commented on a change in pull request #16482:
URL: https://github.com/apache/superset/pull/16482#discussion_r699201604
##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts
##########
@@ -67,9 +67,10 @@ export class OptionSelector {
return !!this.getValues()?.includes(value);
}
- getValues(): string[] | string | undefined {
+ getValues(): string[] | undefined {
+ // ensure an array type return value
if (!this.multi) {
- return this.values.length > 0 ? this.values[0].column_name : undefined;
+ return this.values.length > 0 ? [this.values[0].column_name] : undefined;
Review comment:
i think it's not really about compatibility between dnd and non-dnd, but what chart's logic expects - I think charts expect a string for a single value controls and an array for multi value controls. I tested this PR with Bubble Chart and I'm getting an error after changing a value in a non-multi control
CC @zhaoyongjie
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org