ktmud commented on a change in pull request #13340:
URL: https://github.com/apache/superset/pull/13340#discussion_r585015220



##########
File path: 
superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
##########
@@ -16,19 +16,60 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ColumnMeta } from '@superset-ui/chart-controls';
+import { ReactNode } from 'react';
+import { AdhocFilter } from '@superset-ui/core';
+import {
+  BaseControlConfig,
+  ColumnMeta,
+  Metric,
+} from '@superset-ui/chart-controls';
+import { DatasourcePanelDndItem } from '../../DatasourcePanel/types';
+
+export const GroupByItemType = 'groupByItem';
+export const FilterItemType = 'filterItemType';

Review comment:
       https://github.com/apache/superset/pull/13210/files#r583265905
   
   I'd imagine you will be adding more types in the future, so it could make 
sense to make this an enum.

##########
File path: 
superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts
##########
@@ -19,26 +19,26 @@
 import { ColumnMeta } from '@superset-ui/chart-controls';
 
 export class OptionSelector {

Review comment:
       https://github.com/apache/superset/pull/13210/files#r583272323
   
   Here's one way to make this generic so that adhoc columns/metrics can also 
be selected. But of course you can also make sure each adhoc item have a unique 
id/label and use is as the option key.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to