From 68de69ec2bfaaad38a0cbcb0580e65e97eee0e14 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 16 Jun 2025 15:17:04 +0200 Subject: [PATCH 1/4] refactor: turn cell render into a functional component --- .../components/table-view/cell-renderer.tsx | 429 ++++++++---------- 1 file changed, 196 insertions(+), 233 deletions(-) diff --git a/packages/compass-crud/src/components/table-view/cell-renderer.tsx b/packages/compass-crud/src/components/table-view/cell-renderer.tsx index 86f7051852d..7e60bd27a0d 100644 --- a/packages/compass-crud/src/components/table-view/cell-renderer.tsx +++ b/packages/compass-crud/src/components/table-view/cell-renderer.tsx @@ -1,5 +1,4 @@ import React from 'react'; -import PropTypes from 'prop-types'; import { BSONValue, css, @@ -10,7 +9,6 @@ import { withDarkMode, } from '@mongodb-js/compass-components'; import { Element } from 'hadron-document'; -import type { ICellRendererReactComp } from 'ag-grid-react'; import type { ICellRendererParams } from 'ag-grid-community'; import type { GridActions, TableHeaderType } from '../../stores/grid-store'; import type { CrudActions } from '../../stores/crud-store'; @@ -95,168 +93,148 @@ export type CellRendererProps = Omit & { /** * The custom cell renderer that renders a cell in the table view. */ -class CellRenderer - extends React.Component - implements ICellRendererReactComp -{ - element: Element; - isEmpty: boolean; - isDeleted: boolean; - editable: boolean; - - constructor(props: CellRendererProps) { - super(props); - - this.isEmpty = props.value === undefined || props.value === null; - this.isDeleted = false; - this.element = props.value; - +const CellRenderer: React.FC = ({ + value, + context, + column, + node, + parentType, + elementAdded, + elementRemoved, + elementTypeChanged, + drillDown, + api, + darkMode, +}) => { + const element = value as Element; + + const isEmpty = element === undefined || element === null; + const [isDeleted, setIsDeleted] = React.useState(false); + const [, forceUpdate] = React.useReducer((x) => x + 1, 0); + + const isEditable = React.useMemo(() => { /* Can't get the editable() function from here, so have to reevaluate */ - this.editable = true; - if (props.context.path.length > 0 && props.column.getColId() !== '$_id') { - const parent = props.node.data.hadronDocument.getChild( - props.context.path - ); - if ( - !parent || - (props.parentType && parent.currentType !== props.parentType) - ) { - this.editable = false; + let editable = true; + if (context.path.length > 0 && column.getColId() !== '$_id') { + const parent = node.data.hadronDocument.getChild(context.path); + if (!parent || (parentType && parent.currentType !== parentType)) { + editable = false; } else if (parent.currentType === 'Array') { let maxKey = 0; if (parent.elements.lastElement) { maxKey = +parent.elements.lastElement.currentKey + 1; } - if (+props.column.getColId() > maxKey) { - this.editable = false; + if (+column.getColId() > maxKey) { + editable = false; } } } - } - - componentDidMount() { - if (!this.isEmpty) { - this.subscribeElementEvents(); - } - } - - componentWillUnmount() { - if (!this.isEmpty) { - this.unsubscribeElementEvents(); - } - } - - subscribeElementEvents() { - this.element.on(Element.Events.Added, this.handleElementEvent); - this.element.on(Element.Events.Converted, this.handleElementEvent); - this.element.on(Element.Events.Edited, this.handleElementEvent); - this.element.on(Element.Events.Reverted, this.handleElementEvent); - } - - unsubscribeElementEvents() { - this.element.removeListener(Element.Events.Added, this.handleElementEvent); - this.element.removeListener( - Element.Events.Converted, - this.handleElementEvent - ); - this.element.removeListener(Element.Events.Edited, this.handleElementEvent); - this.element.removeListener( - Element.Events.Reverted, - this.handleElementEvent - ); - } - - handleElementEvent = () => { - this.forceUpdate(); - }; - - handleUndo = (event: React.MouseEvent) => { - event.stopPropagation(); - const oid = this.props.node.data.hadronDocument.getStringId(); - if (this.element.isAdded()) { - this.isDeleted = true; - const isArray = - !this.element.parent?.isRoot() && - this.element.parent?.currentType === 'Array'; - this.props.elementRemoved(String(this.element.currentKey), oid, isArray); - } else if (this.element.isRemoved()) { - this.props.elementAdded( - String(this.element.currentKey), - this.element.currentType, - oid - ); - } else { - this.props.elementTypeChanged( - String(this.element.currentKey), - this.element.type, - oid - ); + return editable; + }, [context.path, column, node.data.hadronDocument, parentType]); + + const handleElementEvent = React.useCallback(() => { + forceUpdate(); + }, []); + + // Subscribe to element events + React.useEffect(() => { + if (!isEmpty && element) { + element.on(Element.Events.Added, handleElementEvent); + element.on(Element.Events.Converted, handleElementEvent); + element.on(Element.Events.Edited, handleElementEvent); + element.on(Element.Events.Reverted, handleElementEvent); + + return () => { + element.removeListener(Element.Events.Added, handleElementEvent); + element.removeListener(Element.Events.Converted, handleElementEvent); + element.removeListener(Element.Events.Edited, handleElementEvent); + element.removeListener(Element.Events.Reverted, handleElementEvent); + }; } - this.element.revert(); - }; - - handleDrillDown(event: React.MouseEvent) { - event.stopPropagation(); - this.props.drillDown(this.props.node.data.hadronDocument, this.element); - } - - handleClicked() { - if (this.props.node.data.state === 'editing') { - this.props.api.startEditingCell({ - rowIndex: this.props.node.rowIndex, - colKey: this.props.column.getColId(), + }, [isEmpty, element, handleElementEvent]); + + const handleUndo = React.useCallback( + (event: React.MouseEvent) => { + event.stopPropagation(); + const oid: string = node.data.hadronDocument.getStringId(); + if (element.isAdded()) { + setIsDeleted(true); + const isArray = + !element.parent?.isRoot() && element.parent?.currentType === 'Array'; + elementRemoved(String(element.currentKey), oid, isArray); + } else if (element.isRemoved()) { + elementAdded(String(element.currentKey), element.currentType, oid); + } else { + elementTypeChanged(String(element.currentKey), element.type, oid); + } + element.revert(); + }, + [ + element, + node.data.hadronDocument, + elementRemoved, + elementAdded, + elementTypeChanged, + ] + ); + + const handleDrillDown = React.useCallback( + (event: React.MouseEvent) => { + event.stopPropagation(); + drillDown(node.data.hadronDocument, element); + }, + [drillDown, node.data.hadronDocument, element] + ); + + const handleClicked = React.useCallback(() => { + if (node.data.state === 'editing') { + api.startEditingCell({ + rowIndex: node.rowIndex, + colKey: column.getColId(), }); } - } + }, [node, api, column]); - refresh() { - return true; - } - - renderInvalidCell() { - let valueClass = `${VALUE_CLASS}-is-${this.element.currentType.toLowerCase()}`; + const renderInvalidCell = React.useCallback(() => { + let valueClass = `${VALUE_CLASS}-is-${element.currentType.toLowerCase()}`; valueClass = `${valueClass} ${INVALID_VALUE}`; - /* Return internal div because invalid cells should only hightlight text? */ - - return
{this.element.currentValue}
; - } + return
{element.currentValue}
; + }, [element]); - getLength(): number | undefined { - if (this.element.currentType === 'Object') { - return Object.keys(this.element.generateObject() as object).length; + const getLength = React.useCallback((): number | undefined => { + if (element.currentType === 'Object') { + return Object.keys(element.generateObject() as object).length; } - if (this.element.currentType === 'Array') { - return this.element.elements!.size; + if (element.currentType === 'Array' && element.elements) { + return element.elements.size; } - } + }, [element]); - renderValidCell() { + const renderValidCell = React.useCallback(() => { let className = VALUE_BASE; - let element: string | JSX.Element = ''; - if (this.element.isAdded()) { + let elementContent: string | JSX.Element = ''; + if (element.isAdded()) { className = `${className} ${VALUE_BASE}-${ADDED}`; - } else if (this.element.isEdited()) { + } else if (element.isEdited()) { className = `${className} ${VALUE_BASE}-${EDITED}`; } - if (this.element.currentType === 'Object') { - element = `{} ${this.getLength() as number} fields`; - } else if (this.element.currentType === 'Array') { - element = `[] ${this.getLength() as number} elements`; + if (element.currentType === 'Object') { + elementContent = `{} ${getLength() as number} fields`; + } else if (element.currentType === 'Array') { + elementContent = `[] ${getLength() as number} elements`; } else { - element = ( - + elementContent = ( + //@ts-expect-error Types for this are currently not consistent + ); } return (
- {this.props.value.decrypted && ( + {element.decrypted && ( )} - {element} + {elementContent}
); - } - - renderUndo(canUndo: boolean, canExpand: boolean) { - let undoButtonClass = `${BUTTON_CLASS} ${BUTTON_CLASS}-undo`; - if (canUndo && canExpand) { - undoButtonClass = `${undoButtonClass} ${BUTTON_CLASS}-left`; - } + }, [element, getLength]); - if (!canUndo) { - return null; - } - return ( - - - - ); - } + const renderUndo = React.useCallback( + (canUndo: boolean, canExpand: boolean) => { + let undoButtonClass = `${BUTTON_CLASS} ${BUTTON_CLASS}-undo`; + if (canUndo && canExpand) { + undoButtonClass = `${undoButtonClass} ${BUTTON_CLASS}-left`; + } - renderExpand(canExpand: boolean) { - if (!canExpand) { - return null; - } - return ( - + if (!canUndo) { + return null; + } + return ( - + - - ); - } - - render() { - let element; - let className = BEM_BASE; - let canUndo = false; - let canExpand = false; - - if (!this.editable) { - element = ''; - className = `${className}-${UNEDITABLE}`; - } else if (this.isEmpty || this.isDeleted) { - element = 'No field'; - className = `${className}-${EMPTY}`; - } else if (!this.element.isCurrentTypeValid()) { - element = this.renderInvalidCell(); - className = `${className}-${INVALID}`; + ); + }, + [handleUndo] + ); + + const renderExpand = React.useCallback( + (canExpand: boolean) => { + if (!canExpand) { + return null; + } + return ( + + + + + + ); + }, + [handleDrillDown] + ); + + // Render logic + let elementToRender; + let className = BEM_BASE; + let canUndo = false; + let canExpand = false; + + if (!isEditable) { + elementToRender = ''; + className = `${className}-${UNEDITABLE}`; + } else if (isEmpty || isDeleted) { + elementToRender = 'No field'; + className = `${className}-${EMPTY}`; + } else if (!element.isCurrentTypeValid()) { + elementToRender = renderInvalidCell(); + className = `${className}-${INVALID}`; + canUndo = true; + } else if (element.isRemoved()) { + elementToRender = 'Deleted field'; + className = `${className}-${DELETED}`; + canUndo = true; + } else { + elementToRender = renderValidCell(); + if (element.isAdded()) { + className = `${className}-${ADDED}`; canUndo = true; - } else if (this.element.isRemoved()) { - element = 'Deleted field'; - className = `${className}-${DELETED}`; + } else if (element.isModified()) { + className = `${className}-${EDITED}`; canUndo = true; - } else { - element = this.renderValidCell(); - if (this.element.isAdded()) { - className = `${className}-${ADDED}`; - canUndo = true; - } else if (this.element.isModified()) { - className = `${className}-${EDITED}`; - canUndo = true; - } - canExpand = - this.element.currentType === 'Object' || - this.element.currentType === 'Array'; } + canExpand = + element.currentType === 'Object' || element.currentType === 'Array'; + } - return ( - // `ag-grid` renders this component outside of the context chain - // so we re-supply the dark mode theme here. - + return ( + // `ag-grid` renders this component outside of the context chain + // so we re-supply the dark mode theme here. + +
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/interactive-supports-focus*/} -
- {this.renderUndo(canUndo, canExpand)} - {this.renderExpand(canExpand)} - {element} +
+ {renderUndo(canUndo, canExpand)} + {renderExpand(canExpand)} + {elementToRender}
- - ); - } - - static propTypes = { - api: PropTypes.any, - value: PropTypes.any, - node: PropTypes.any, - column: PropTypes.any, - context: PropTypes.any, - parentType: PropTypes.any.isRequired, - elementAdded: PropTypes.func.isRequired, - elementRemoved: PropTypes.func.isRequired, - elementTypeChanged: PropTypes.func.isRequired, - drillDown: PropTypes.func.isRequired, - tz: PropTypes.string.isRequired, - darkMode: PropTypes.bool, - }; - - static displayName = 'CellRenderer'; -} +
+ + ); +}; export default withDarkMode(CellRenderer); From f31490c700be53fd1f4bd18be8f68903aac56995 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 16 Jun 2025 18:21:31 +0200 Subject: [PATCH 2/4] refactor: untangle usage of nested if statements --- .../components/table-view/cell-renderer.tsx | 398 +++++++++++------- 1 file changed, 237 insertions(+), 161 deletions(-) diff --git a/packages/compass-crud/src/components/table-view/cell-renderer.tsx b/packages/compass-crud/src/components/table-view/cell-renderer.tsx index 7e60bd27a0d..4af6d3ada32 100644 --- a/packages/compass-crud/src/components/table-view/cell-renderer.tsx +++ b/packages/compass-crud/src/components/table-view/cell-renderer.tsx @@ -1,4 +1,10 @@ -import React from 'react'; +import React, { + useMemo, + useCallback, + useEffect, + useReducer, + useState, +} from 'react'; import { BSONValue, css, @@ -8,7 +14,7 @@ import { spacing, withDarkMode, } from '@mongodb-js/compass-components'; -import { Element } from 'hadron-document'; +import { type Document, Element } from 'hadron-document'; import type { ICellRendererParams } from 'ag-grid-community'; import type { GridActions, TableHeaderType } from '../../stores/grid-store'; import type { CrudActions } from '../../stores/crud-store'; @@ -59,6 +65,11 @@ const UNEDITABLE = 'is-uneditable'; */ const INVALID = 'is-invalid'; +/** + * The valid constant. + */ +const VALID = 'valid'; + /** * The deleted constant. */ @@ -79,6 +90,141 @@ const decrypdedIconStyles = css({ display: 'flex', }); +interface CellContentProps { + element: Element | undefined | null; + cellState: + | typeof UNEDITABLE + | typeof EMPTY + | typeof INVALID + | typeof DELETED + | typeof ADDED + | typeof EDITED + | typeof VALID; + onUndo: (event: React.MouseEvent) => void; + onExpand: (event: React.MouseEvent) => void; +} + +const CellContent: React.FC = ({ + element, + cellState, + onUndo, + onExpand, +}) => { + const [, forceUpdate] = useReducer((x: number) => x + 1, 0); + const isEmpty = element === undefined || element === null; + const handleElementEvent = useCallback(() => { + forceUpdate(); + }, []); + + // Subscribe to element events + useEffect(() => { + if (!isEmpty && element) { + element.on(Element.Events.Added, handleElementEvent); + element.on(Element.Events.Converted, handleElementEvent); + element.on(Element.Events.Edited, handleElementEvent); + element.on(Element.Events.Reverted, handleElementEvent); + + return () => { + element.removeListener(Element.Events.Added, handleElementEvent); + element.removeListener(Element.Events.Converted, handleElementEvent); + element.removeListener(Element.Events.Edited, handleElementEvent); + element.removeListener(Element.Events.Reverted, handleElementEvent); + }; + } + }, [isEmpty, element, handleElementEvent]); + + const elementLength = useMemo((): number | undefined => { + if (!element) { + return undefined; + } + + if (element.currentType === 'Object') { + return Object.keys(element.generateObject() as object).length; + } + if (element.currentType === 'Array' && element.elements) { + return element.elements.size; + } + }, [element]); + + const renderContent = useCallback(() => { + if (cellState === EMPTY || !element) { + return 'No field'; + } + + if (cellState === UNEDITABLE) { + return ''; + } + + if (cellState === DELETED) { + return 'Deleted field'; + } + + if (cellState === INVALID) { + let valueClass = `${VALUE_CLASS}-is-${element.currentType.toLowerCase()}`; + valueClass = `${valueClass} ${INVALID_VALUE}`; + + return
{element.currentValue}
; + } + + let className = VALUE_BASE; + let elementContent: string | JSX.Element = ''; + if (cellState === ADDED || cellState === EDITED) { + className = `${className} ${VALUE_BASE}-${cellState}`; + } + + const isArrayOrObject = + element.currentType === 'Array' || element.currentType === 'Object'; + + if (elementLength !== undefined && isArrayOrObject) { + if (element.currentType === 'Object') { + elementContent = `{} ${elementLength} fields`; + } else if (element.currentType === 'Array') { + elementContent = `[] ${elementLength} elements`; + } + } else { + elementContent = ( + //@ts-expect-error Types for this are currently not consistent + + ); + } + + return ( +
+
+ {element.decrypted && ( + + + + )} + {elementContent} +
+
+ ); + }, [element, elementLength, cellState]); + + const canUndo = + cellState === ADDED || + cellState === EDITED || + cellState === INVALID || + cellState === DELETED; + + const canExpand = + (cellState === VALID || cellState === ADDED || cellState === EDITED) && + (element?.currentType === 'Object' || element?.currentType === 'Array'); + + return ( + <> + {canUndo && } + {canExpand && } + {renderContent()} + + ); +}; + export type CellRendererProps = Omit & { context: GridContext; parentType: TableHeaderType; @@ -106,13 +252,12 @@ const CellRenderer: React.FC = ({ api, darkMode, }) => { - const element = value as Element; + const element = value as Element | undefined | null; const isEmpty = element === undefined || element === null; - const [isDeleted, setIsDeleted] = React.useState(false); - const [, forceUpdate] = React.useReducer((x) => x + 1, 0); + const [isDeleted, setIsDeleted] = useState(false); - const isEditable = React.useMemo(() => { + const isEditable = useMemo(() => { /* Can't get the editable() function from here, so have to reevaluate */ let editable = true; if (context.path.length > 0 && column.getColId() !== '$_id') { @@ -132,30 +277,12 @@ const CellRenderer: React.FC = ({ return editable; }, [context.path, column, node.data.hadronDocument, parentType]); - const handleElementEvent = React.useCallback(() => { - forceUpdate(); - }, []); - - // Subscribe to element events - React.useEffect(() => { - if (!isEmpty && element) { - element.on(Element.Events.Added, handleElementEvent); - element.on(Element.Events.Converted, handleElementEvent); - element.on(Element.Events.Edited, handleElementEvent); - element.on(Element.Events.Reverted, handleElementEvent); - - return () => { - element.removeListener(Element.Events.Added, handleElementEvent); - element.removeListener(Element.Events.Converted, handleElementEvent); - element.removeListener(Element.Events.Edited, handleElementEvent); - element.removeListener(Element.Events.Reverted, handleElementEvent); - }; - } - }, [isEmpty, element, handleElementEvent]); - - const handleUndo = React.useCallback( + const handleUndo = useCallback( (event: React.MouseEvent) => { event.stopPropagation(); + if (!element) { + return; + } const oid: string = node.data.hadronDocument.getStringId(); if (element.isAdded()) { setIsDeleted(true); @@ -178,15 +305,18 @@ const CellRenderer: React.FC = ({ ] ); - const handleDrillDown = React.useCallback( + const handleDrillDown = useCallback( (event: React.MouseEvent) => { event.stopPropagation(); - drillDown(node.data.hadronDocument, element); + if (!element) { + return; + } + drillDown(node.data.hadronDocument as Document, element); }, [drillDown, node.data.hadronDocument, element] ); - const handleClicked = React.useCallback(() => { + const handleClicked = useCallback(() => { if (node.data.state === 'editing') { api.startEditingCell({ rowIndex: node.rowIndex, @@ -195,138 +325,30 @@ const CellRenderer: React.FC = ({ } }, [node, api, column]); - const renderInvalidCell = React.useCallback(() => { - let valueClass = `${VALUE_CLASS}-is-${element.currentType.toLowerCase()}`; - valueClass = `${valueClass} ${INVALID_VALUE}`; - - return
{element.currentValue}
; - }, [element]); - - const getLength = React.useCallback((): number | undefined => { - if (element.currentType === 'Object') { - return Object.keys(element.generateObject() as object).length; - } - if (element.currentType === 'Array' && element.elements) { - return element.elements.size; - } - }, [element]); - - const renderValidCell = React.useCallback(() => { - let className = VALUE_BASE; - let elementContent: string | JSX.Element = ''; - if (element.isAdded()) { - className = `${className} ${VALUE_BASE}-${ADDED}`; - } else if (element.isEdited()) { - className = `${className} ${VALUE_BASE}-${EDITED}`; - } - - if (element.currentType === 'Object') { - elementContent = `{} ${getLength() as number} fields`; - } else if (element.currentType === 'Array') { - elementContent = `[] ${getLength() as number} elements`; - } else { - elementContent = ( - //@ts-expect-error Types for this are currently not consistent - - ); - } - - return ( -
-
- {element.decrypted && ( - - - - )} - {elementContent} -
-
- ); - }, [element, getLength]); - - const renderUndo = React.useCallback( - (canUndo: boolean, canExpand: boolean) => { - let undoButtonClass = `${BUTTON_CLASS} ${BUTTON_CLASS}-undo`; - if (canUndo && canExpand) { - undoButtonClass = `${undoButtonClass} ${BUTTON_CLASS}-left`; - } - - if (!canUndo) { - return null; - } - return ( - - - - ); - }, - [handleUndo] - ); - - const renderExpand = React.useCallback( - (canExpand: boolean) => { - if (!canExpand) { - return null; - } - return ( - - - - - - ); - }, - [handleDrillDown] - ); - - // Render logic - let elementToRender; - let className = BEM_BASE; - let canUndo = false; - let canExpand = false; + // Determine cell state + let cellState: + | typeof UNEDITABLE + | typeof EMPTY + | typeof INVALID + | typeof DELETED + | typeof ADDED + | typeof EDITED + | typeof VALID; if (!isEditable) { - elementToRender = ''; - className = `${className}-${UNEDITABLE}`; + cellState = UNEDITABLE; } else if (isEmpty || isDeleted) { - elementToRender = 'No field'; - className = `${className}-${EMPTY}`; + cellState = EMPTY; } else if (!element.isCurrentTypeValid()) { - elementToRender = renderInvalidCell(); - className = `${className}-${INVALID}`; - canUndo = true; + cellState = INVALID; } else if (element.isRemoved()) { - elementToRender = 'Deleted field'; - className = `${className}-${DELETED}`; - canUndo = true; + cellState = DELETED; + } else if (element.isAdded()) { + cellState = ADDED; + } else if (element.isModified()) { + cellState = EDITED; } else { - elementToRender = renderValidCell(); - if (element.isAdded()) { - className = `${className}-${ADDED}`; - canUndo = true; - } else if (element.isModified()) { - className = `${className}-${EDITED}`; - canUndo = true; - } - canExpand = - element.currentType === 'Object' || element.currentType === 'Array'; + cellState = VALID; } return ( @@ -335,10 +357,19 @@ const CellRenderer: React.FC = ({
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/interactive-supports-focus*/} -
- {renderUndo(canUndo, canExpand)} - {renderExpand(canExpand)} - {elementToRender} +
+
@@ -346,3 +377,48 @@ const CellRenderer: React.FC = ({ }; export default withDarkMode(CellRenderer); + +interface CellUndoButtonProps { + alignLeft: boolean; + onClick: (event: React.MouseEvent) => void; +} + +const CellUndoButton: React.FC = ({ + alignLeft, + onClick, +}) => { + let undoButtonClass = `${BUTTON_CLASS} ${BUTTON_CLASS}-undo`; + if (alignLeft) { + undoButtonClass = `${undoButtonClass} ${BUTTON_CLASS}-left`; + } + + return ( + + + + ); +}; + +interface CellExpandButtonProps { + onClick: (event: React.MouseEvent) => void; +} + +const CellExpandButton: React.FC = ({ onClick }) => { + return ( + + + + ); +}; From b45d8fa21c72784de24289d8c5dd127203a6255a Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 17 Jun 2025 13:13:53 +0200 Subject: [PATCH 3/4] fix: use CellState wherever possible --- .../components/table-view/cell-renderer.tsx | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/compass-crud/src/components/table-view/cell-renderer.tsx b/packages/compass-crud/src/components/table-view/cell-renderer.tsx index 4af6d3ada32..32ca17e889f 100644 --- a/packages/compass-crud/src/components/table-view/cell-renderer.tsx +++ b/packages/compass-crud/src/components/table-view/cell-renderer.tsx @@ -277,6 +277,32 @@ const CellRenderer: React.FC = ({ return editable; }, [context.path, column, node.data.hadronDocument, parentType]); + // Determine cell state + let cellState: + | typeof UNEDITABLE + | typeof EMPTY + | typeof INVALID + | typeof DELETED + | typeof ADDED + | typeof EDITED + | typeof VALID; + + if (!isEditable) { + cellState = UNEDITABLE; + } else if (isEmpty || isDeleted) { + cellState = EMPTY; + } else if (!element.isCurrentTypeValid()) { + cellState = INVALID; + } else if (element.isRemoved()) { + cellState = DELETED; + } else if (element.isAdded()) { + cellState = ADDED; + } else if (element.isModified()) { + cellState = EDITED; + } else { + cellState = VALID; + } + const handleUndo = useCallback( (event: React.MouseEvent) => { event.stopPropagation(); @@ -284,12 +310,12 @@ const CellRenderer: React.FC = ({ return; } const oid: string = node.data.hadronDocument.getStringId(); - if (element.isAdded()) { + if (cellState === ADDED) { setIsDeleted(true); const isArray = !element.parent?.isRoot() && element.parent?.currentType === 'Array'; elementRemoved(String(element.currentKey), oid, isArray); - } else if (element.isRemoved()) { + } else if (cellState === DELETED) { elementAdded(String(element.currentKey), element.currentType, oid); } else { elementTypeChanged(String(element.currentKey), element.type, oid); @@ -325,32 +351,6 @@ const CellRenderer: React.FC = ({ } }, [node, api, column]); - // Determine cell state - let cellState: - | typeof UNEDITABLE - | typeof EMPTY - | typeof INVALID - | typeof DELETED - | typeof ADDED - | typeof EDITED - | typeof VALID; - - if (!isEditable) { - cellState = UNEDITABLE; - } else if (isEmpty || isDeleted) { - cellState = EMPTY; - } else if (!element.isCurrentTypeValid()) { - cellState = INVALID; - } else if (element.isRemoved()) { - cellState = DELETED; - } else if (element.isAdded()) { - cellState = ADDED; - } else if (element.isModified()) { - cellState = EDITED; - } else { - cellState = VALID; - } - return ( // `ag-grid` renders this component outside of the context chain // so we re-supply the dark mode theme here. From 077e368e83d989159d88085d222e944b4069705d Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 22:09:25 +0200 Subject: [PATCH 4/4] fix: changes from feedback --- .../src/components/document-list/document.tsx | 2 +- .../src/components/document-list/element.tsx | 2 +- .../use-force-update.tsx | 0 packages/compass-components/src/index.ts | 1 + .../components/table-view/cell-renderer.tsx | 77 ++++++++++--------- 5 files changed, 42 insertions(+), 40 deletions(-) rename packages/compass-components/src/{components/document-list => hooks}/use-force-update.tsx (100%) diff --git a/packages/compass-components/src/components/document-list/document.tsx b/packages/compass-components/src/components/document-list/document.tsx index 2e2d9cb540c..705af18acd6 100644 --- a/packages/compass-components/src/components/document-list/document.tsx +++ b/packages/compass-components/src/components/document-list/document.tsx @@ -10,7 +10,7 @@ import { ElementEvents, } from 'hadron-document'; import { AutoFocusContext } from './auto-focus-context'; -import { useForceUpdate } from './use-force-update'; +import { useForceUpdate } from '../../hooks/use-force-update'; import { calculateShowMoreToggleOffset, HadronElement } from './element'; import { usePrevious } from './use-previous'; import VisibleFieldsToggle from './visible-field-toggle'; diff --git a/packages/compass-components/src/components/document-list/element.tsx b/packages/compass-components/src/components/document-list/element.tsx index b79c96f4015..49cf8e277d5 100644 --- a/packages/compass-components/src/components/document-list/element.tsx +++ b/packages/compass-components/src/components/document-list/element.tsx @@ -21,7 +21,7 @@ import { spacing } from '@leafygreen-ui/tokens'; import { KeyEditor, ValueEditor, TypeEditor } from './element-editors'; import { EditActions, AddFieldActions } from './element-actions'; import { useAutoFocusContext } from './auto-focus-context'; -import { useForceUpdate } from './use-force-update'; +import { useForceUpdate } from '../../hooks/use-force-update'; import { usePrevious } from './use-previous'; import { css, cx } from '@leafygreen-ui/emotion'; import { palette } from '@leafygreen-ui/palette'; diff --git a/packages/compass-components/src/components/document-list/use-force-update.tsx b/packages/compass-components/src/hooks/use-force-update.tsx similarity index 100% rename from packages/compass-components/src/components/document-list/use-force-update.tsx rename to packages/compass-components/src/hooks/use-force-update.tsx diff --git a/packages/compass-components/src/index.ts b/packages/compass-components/src/index.ts index d577dd2bf13..2fb943dda18 100644 --- a/packages/compass-components/src/index.ts +++ b/packages/compass-components/src/index.ts @@ -213,3 +213,4 @@ export { export { SelectList } from './components/select-list'; export { ParagraphSkeleton } from '@leafygreen-ui/skeleton-loader'; export { InsightsChip } from './components/insights-chip'; +export { useForceUpdate } from './hooks/use-force-update'; diff --git a/packages/compass-crud/src/components/table-view/cell-renderer.tsx b/packages/compass-crud/src/components/table-view/cell-renderer.tsx index 32ca17e889f..8526fd9c426 100644 --- a/packages/compass-crud/src/components/table-view/cell-renderer.tsx +++ b/packages/compass-crud/src/components/table-view/cell-renderer.tsx @@ -1,10 +1,4 @@ -import React, { - useMemo, - useCallback, - useEffect, - useReducer, - useState, -} from 'react'; +import React, { useMemo, useCallback, useEffect, useState } from 'react'; import { BSONValue, css, @@ -13,6 +7,7 @@ import { LeafyGreenProvider, spacing, withDarkMode, + useForceUpdate, } from '@mongodb-js/compass-components'; import { type Document, Element } from 'hadron-document'; import type { ICellRendererParams } from 'ag-grid-community'; @@ -90,6 +85,21 @@ const decrypdedIconStyles = css({ display: 'flex', }); +const getElementLength = ( + element: Element | undefined | null +): number | undefined => { + if (!element) { + return undefined; + } + + if (element.currentType === 'Object') { + return Object.keys(element.generateObject() as object).length; + } + if (element.currentType === 'Array' && element.elements) { + return element.elements.size; + } +}; + interface CellContentProps { element: Element | undefined | null; cellState: @@ -110,7 +120,7 @@ const CellContent: React.FC = ({ onUndo, onExpand, }) => { - const [, forceUpdate] = useReducer((x: number) => x + 1, 0); + const forceUpdate = useForceUpdate(); const isEmpty = element === undefined || element === null; const handleElementEvent = useCallback(() => { forceUpdate(); @@ -133,18 +143,7 @@ const CellContent: React.FC = ({ } }, [isEmpty, element, handleElementEvent]); - const elementLength = useMemo((): number | undefined => { - if (!element) { - return undefined; - } - - if (element.currentType === 'Object') { - return Object.keys(element.generateObject() as object).length; - } - if (element.currentType === 'Array' && element.elements) { - return element.elements.size; - } - }, [element]); + const elementLength = getElementLength(element); const renderContent = useCallback(() => { if (cellState === EMPTY || !element) { @@ -183,7 +182,6 @@ const CellContent: React.FC = ({ } } else { elementContent = ( - //@ts-expect-error Types for this are currently not consistent ); } @@ -204,7 +202,14 @@ const CellContent: React.FC = ({
); - }, [element, elementLength, cellState]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + element, + element?.currentType, + element?.currentValue, + elementLength, + cellState, + ]); const canUndo = cellState === ADDED || @@ -355,22 +360,18 @@ const CellRenderer: React.FC = ({ // `ag-grid` renders this component outside of the context chain // so we re-supply the dark mode theme here. -
- {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/interactive-supports-focus*/} -
- -
+ {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/interactive-supports-focus*/} +
+
);