kgabryje commented on code in PR #26155: URL: https://github.com/apache/superset/pull/26155#discussion_r1434995476
########## superset-frontend/src/components/AlteredSliceTag/index.jsx: ########## @@ -60,39 +61,73 @@ function alterForComparison(value) { } } return value; -} - -export default class AlteredSliceTag extends React.Component { - constructor(props) { - super(props); - const diffs = this.getDiffs(props); - const controlsMap = getControlsForVizType(this.props.origFormData.viz_type); - const rows = this.getRowsFromDiffs(diffs, controlsMap); +}; - this.state = { rows, hasDiffs: !isEmpty(diffs), controlsMap }; +export const formatValueHandler = (value, key, controlsMap) => { + // Format display value based on the control type + // or the value type + if (value === undefined) { + return 'N/A'; } - - UNSAFE_componentWillReceiveProps(newProps) { - // Update differences if need be - if (isEqual(this.props, newProps)) { - return; + if (value === null) { + return 'null'; + } + if (controlsMap[key]?.type === 'AdhocFilterControl') { + if (!value.length) { + return '[]'; } - const diffs = this.getDiffs(newProps); - this.setState(prevState => ({ - rows: this.getRowsFromDiffs(diffs, prevState.controlsMap), - hasDiffs: !isEmpty(diffs), - })); + return value + .map(v => { + const filterVal = + v.comparator && v.comparator.constructor === Array + ? `[${v.comparator.join(', ')}]` + : v.comparator; + return `${v.subject} ${v.operator} ${filterVal}`; + }) + .join(', '); } - - getRowsFromDiffs(diffs, controlsMap) { - return Object.entries(diffs).map(([key, diff]) => ({ - control: (controlsMap[key] && controlsMap[key].label) || key, - before: this.formatValue(diff.before, key, controlsMap), - after: this.formatValue(diff.after, key, controlsMap), - })); + if (controlsMap[key]?.type === 'BoundsControl') { + return `Min: ${value[0]}, Max: ${value[1]}`; + } + if (controlsMap[key]?.type === 'CollectionControl') { + return value.map(v => safeStringify(v)).join(', '); + } + if ( + controlsMap[key]?.type === 'MetricsControl' && + value.constructor === Array + ) { + const formattedValue = value.map(v => v?.label ?? v); + return formattedValue.length ? formattedValue.join(', ') : '[]'; + } + if (typeof value === 'boolean') { + return value ? 'true' : 'false'; + } + if (value.constructor === Array) { + const formattedValue = value.map(v => v?.label ?? v); + return formattedValue.length ? formattedValue.join(', ') : '[]'; + } + if (typeof value === 'string' || typeof value === 'number') { + return value; } + return safeStringify(value); +}; + +export const getRowsFromDiffs = (diffs, controlsMap) => + Object.entries(diffs).map(([key, diff]) => ({ + control: (controlsMap[key] && controlsMap[key].label) || key, + before: formatValueHandler(diff.before, key, controlsMap), + after: formatValueHandler(diff.after, key, controlsMap), + })); + +export const isEqualish = (val1, val2) => + isEqual(alterForComparison(val1), alterForComparison(val2)); - getDiffs(props) { +const AlteredSliceTag = props => { + const prevProps = useRef({}); + const [rows, setRows] = useState([]); + const [hasDiffs, setHasDiffs] = useState(false); + + const getDiffs = useCallback(props => { Review Comment: We can skip `props` argument since this function already has access to props -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org