rusackas commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3478099990
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx:
##########
@@ -102,373 +90,337 @@ const LayerSelectContainer = styled.div`
margin-bottom: ${({ theme }) => theme.marginXXL}px;
`;
-export default class AdhocFilterEditPopover extends Component<
- AdhocFilterEditPopoverProps,
- AdhocFilterEditPopoverState
-> {
- popoverContentRef: RefObject<HTMLDivElement>;
-
- dragStartX = 0;
-
- dragStartY = 0;
-
- dragStartWidth = 0;
-
- dragStartHeight = 0;
-
- constructor(props: AdhocFilterEditPopoverProps) {
- super(props);
- this.onSave = this.onSave.bind(this);
- this.onDragDown = this.onDragDown.bind(this);
- this.onMouseMove = this.onMouseMove.bind(this);
- this.onMouseUp = this.onMouseUp.bind(this);
- this.onAdhocFilterChange = this.onAdhocFilterChange.bind(this);
- this.setSimpleTabIsValid = this.setSimpleTabIsValid.bind(this);
- this.adjustHeight = this.adjustHeight.bind(this);
- this.onTabChange = this.onTabChange.bind(this);
- this.loadLayerOptions = this.loadLayerOptions.bind(this);
- this.onLayerChange = this.onLayerChange.bind(this);
-
- this.state = {
- adhocFilter: this.props.adhocFilter,
- width: POPOVER_INITIAL_WIDTH,
- height: POPOVER_INITIAL_HEIGHT,
- activeKey: this.props?.adhocFilter?.expressionType || 'SIMPLE',
- isSimpleTabValid: true,
- selectedLayers: [{ id: null, value: -1, label: 'All' }],
- layerOptions: [],
- hasLayerFilterScopeChanged: false,
- };
+function AdhocFilterEditPopover({
+ adhocFilter: propsAdhocFilter,
+ onChange,
+ onClose,
+ onResize,
+ options,
+ datasource,
+ partitionColumn,
+ operators,
+ requireSave,
+ ...popoverProps
+}: AdhocFilterEditPopoverProps) {
+ const popoverContentRef = useRef<HTMLDivElement>(null);
+
+ const dragStartRef = useRef({
+ x: 0,
+ y: 0,
+ width: 0,
+ height: 0,
+ });
+
+ const [adhocFilter, setAdhocFilter] =
useState<AdhocFilter>(propsAdhocFilter);
+ const [width, setWidth] = useState(POPOVER_INITIAL_WIDTH);
+ const [height, setHeight] = useState(POPOVER_INITIAL_HEIGHT);
+ const [isSimpleTabValid, setIsSimpleTabValid] = useState(true);
+ const [selectedLayers, setSelectedLayers] = useState<LayerOption[]>([
+ { id: null, value: -1, label: 'All' },
+ ]);
+ const [layerOptions, setLayerOptions] = useState<LayerOption[]>([]);
+ const [hasLayerFilterScopeChanged, setHasLayerFilterScopeChanged] =
+ useState(false);
+
+ const loadLayerOptions = useCallback(
+ (page: number, pageSize: number) => {
+ const query = rison.encode({
+ columns: ['id', 'slice_name', 'viz_type'],
+ filters: [{ col: 'viz_type', opr: 'sw', value: 'deck' }],
+ page,
+ page_size: pageSize,
+ order_column: 'slice_name',
+ order_direction: 'asc',
+ });
- this.popoverContentRef = createRef();
- }
+ return SupersetClient.get({
+ endpoint: `/api/v1/chart/?q=${query}`,
+ }).then(response => {
+ if (!response?.json?.result) {
+ return {
+ data: [
+ {
+ id: null,
+ value: -1,
+ label: 'All',
+ },
+ ],
+ totalCount: 1,
+ };
+ }
+
+ const deckSlices = (propsAdhocFilter?.deck_slices || []) as number[];
+
+ const list = [
+ {
+ id: null,
+ value: -1,
+ label: 'All',
+ },
+ ...response.json.result
+ .map((item: { id: number; slice_name: string }) => {
+ const sliceIndex = deckSlices.indexOf(item.id);
+ return {
+ id: item.id,
+ value: sliceIndex >= 0 ? sliceIndex : item.id,
+ label: item.slice_name,
+ sliceIndex,
+ };
+ })
+ .filter((item: { sliceIndex: number }) => item.sliceIndex !== -1)
+ .map(
+ ({
+ sliceIndex,
+ ...item
+ }: {
+ sliceIndex: number;
+ id: number;
+ value: number;
+ label: string;
+ }) => item,
+ ),
+ ];
- componentDidMount() {
- document.addEventListener('mouseup', this.onMouseUp);
+ return {
+ data: list,
+ totalCount: list.length,
+ };
+ });
+ },
+ [propsAdhocFilter?.deck_slices],
+ );
+
+ const onMouseMove = useCallback(
+ (e: MouseEvent) => {
+ onResize();
+ setWidth(
+ Math.max(
+ dragStartRef.current.width + (e.clientX - dragStartRef.current.x),
+ POPOVER_INITIAL_WIDTH,
+ ),
+ );
+ setHeight(
+ Math.max(
+ dragStartRef.current.height + (e.clientY - dragStartRef.current.y),
+ POPOVER_INITIAL_HEIGHT,
+ ),
+ );
+ },
+ [onResize],
+ );
+
+ const onMouseUp = useCallback(() => {
+ document.removeEventListener('mousemove', onMouseMove);
+ }, [onMouseMove]);
+
+ useEffect(() => {
+ document.addEventListener('mouseup', onMouseUp);
// Load layer options if deck_slices exist
- const deckSlices = this.props.adhocFilter?.deck_slices as
- | number[]
- | undefined;
+ const deckSlices = propsAdhocFilter?.deck_slices as number[] | undefined;
if (deckSlices && deckSlices.length > 0) {
- this.loadLayerOptions(0, 100).then(result => {
- this.setState({ layerOptions: result.data });
- const layerFilterScope = this.props.adhocFilter?.layerFilterScope as
+ loadLayerOptions(0, 100).then(result => {
+ setLayerOptions(result.data);
+ const layerFilterScope = propsAdhocFilter?.layerFilterScope as
| number[]
| undefined;
if (layerFilterScope) {
- const selectedLayers = layerFilterScope.map(item => {
- const layerOption = result.data.find(
- option => option.value === item,
- );
- return layerOption;
- });
- this.setState({
- selectedLayers: selectedLayers.filter(Boolean) as LayerOption[],
- });
+ const layers = layerFilterScope
+ .map(item => result.data.find(option => option.value === item))
+ .filter(Boolean) as LayerOption[];
+ setSelectedLayers(layers);
}
});
}
- }
- componentWillUnmount() {
- document.removeEventListener('mouseup', this.onMouseUp);
- document.removeEventListener('mousemove', this.onMouseMove);
- }
+ return () => {
+ document.removeEventListener('mouseup', onMouseUp);
+ document.removeEventListener('mousemove', onMouseMove);
+ };
+ }, [loadLayerOptions, onMouseMove, onMouseUp, propsAdhocFilter]);
Review Comment:
Split it into two effects - one keyed on `[onMouseUp, onMouseMove]` for the
listener lifecycle, and a `[]` one for the one-time deck-layer options load
(mirroring the old `componentDidMount`), so it no longer rebinds or re-fetches
on every `propsAdhocFilter` change.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
