Skip to content
Snippets Groups Projects

Fix row value typing by introducing new RowValue type

Merged Loic Huder requested to merge fix-value-typing into main

Merge request reports

Merge request pipeline #157559 passed

Merge request pipeline passed for 43b88c8a

Merged by Loic HuderLoic Huder 11 months ago (Feb 6, 2024 8:52am UTC)

Loading

Pipeline #157561 passed

Pipeline passed for c12c8d6f on main

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Loic Huder
  • 4 3
    5 import type { InputTableRow, RowChangeEvent } from '../../../../types';
    4 import type { RowValue } from '../../../../types';
    6 5 import { RowType } from '../../../../types';
    7 6 import { isDecimalNumber } from '../../../../utils/utils';
    8 7 import BooleanControl from './BooleanControl';
    8 import EditJsonButton from './EditJsonButton';
    9 9 import styles from './MultiTypeEditControl.module.css';
    10 10
    11 11 interface Props {
    12 row: InputTableRow;
    13 name: 'name' | 'value';
    14 onChange: (e: RowChangeEvent) => void;
    12 value: RowValue;
    13 type: RowType;
    14 onChange: (newValue: RowValue) => void;
    • In the vein of my previous change, I made low-level components such as this one unaware of the row concept.

      This allowed to me to avoid relying on RowChangeEvent (that has wrong typings as mentionned in !462 (comment 308265)) and instead only type the value itself with the corrected TS type RowValue.

    • Please register or sign in to reply
  • Loic Huder
  • 36 37 const rows = defaultValues.map(createData);
    37 38 const showErrorMsg = useSnackbarStore((state) => state.showErrorMsg);
    38 39
    39 function onChange(e: RowChangeEvent, row: InputTableRow) {
    40 function handleNameChange(e: RowChangeEvent, row: InputTableRow) {
  • 247 247 // width?: number | null; // what is their functionality?
    248 248 // height?: number | null;
    249 249
    250 export type RowValue = string | object | boolean | number | null;
    251
    250 252 export interface InputTableRow {
    251 253 rowId: string;
    252 254 name?: string | number;
    253 value?: unknown;
    255 value?: unknown; // TODO: set to RowValue once Conditions/DefaultInputs types are merged with this
    • Again, this opened a future opportunity of refactoring. value could even be a generic based on type.

      I had my share of refactoring so I think I'll leave it like that for a while.

    • Please register or sign in to reply
  • Loic Huder deleted the improve-editable-table branch. This merge request now targets the main branch

    deleted the improve-editable-table branch. This merge request now targets the main branch

  • Loic Huder marked this merge request as ready

    marked this merge request as ready

  • Loic Huder requested review from @koumouts

    requested review from @koumouts

  • Loic Huder added 9 commits

    added 9 commits

    Compare with previous version

  • mentioned in issue #250 (closed)

  • 35 );
    36 }
    37
    38 if (type === RowType.Number) {
    42 39 return (
    43 40 <FormControl variant="standard" fullWidth>
    44 41 <Input
    45 42 disabled={disable}
    46 value={row[name]}
    43 value={value}
    47 44 type="text"
    48 name={name}
    49 onChange={(event) => onChangeNumber(event)}
    45 onChange={(event) => {
    46 const { value: newValue } = event.target;
    47 if (isDecimalNumber(newValue)) {
  • 36 37 const rows = defaultValues.map(createData);
    37 38 const showErrorMsg = useSnackbarStore((state) => state.showErrorMsg);
    38 39
    39 function onChange(e: RowChangeEvent, row: InputTableRow) {
    40 function handleNameChange(e: RowChangeEvent, row: InputTableRow) {
    40 41 const { rowId: id } = row;
    41 42 const otherRows = rows.filter((_row) => _row.rowId !== id);
    42 43
    43 if (
    44 e.target.name === 'name' &&
    45 otherRows.map((r) => r.name).includes(e.target.value as string)
    46 ) {
    44 if (otherRows.map((r) => r.name).includes(e.target.value as string)) {
    47 45 showErrorMsg('Not allowed to assign the same property TWICE!');
    48 46 // return;
  • 10 onChange: (newValue: RowValue) => void;
    10 11 disable?: boolean;
    11 onChange: (e: RowChangeEvent) => void;
    12 12 }
    13 13
    14 14 function MultiTypeEditCell(props: Props) {
    15 15 const { row, disable, onChange } = props;
    16 16
    17 const { value, type } = row;
    17 const { value = '', type = RowType.String } = row;
    18
    19 if (
    20 typeof value !== 'string' &&
    21 typeof value !== 'object' &&
    22 typeof value !== 'boolean' &&
    23 typeof value !== 'number'
  • 21 typeof value !== 'object' &&
    22 typeof value !== 'boolean' &&
    23 typeof value !== 'number'
    24 ) {
    25 throw new TypeError(
    26 `Expected string, object, boolean or number. Got ${typeof value} instead.`,
    27 );
    28 }
    18 29
    19 30 return (
    20 31 <TableCell
    21 32 align="left"
    22 33 className={styles.cell}
    23 34 data-disabled={disable ? '' : undefined}
    24 data-invalid={value === undefined || value === '' ? '' : undefined}
    35 data-invalid={value === '' ? '' : undefined}
  • Giannis Koumoutsos approved this merge request

    approved this merge request

  • Loic Huder added 1 commit

    added 1 commit

    • 43b88c8a - Fix row value typing by introducing new RowValue type

    Compare with previous version

  • Loic Huder enabled an automatic merge when the pipeline for 43b88c8a succeeds

    enabled an automatic merge when the pipeline for 43b88c8a succeeds

  • merged

  • Loic Huder mentioned in commit c12c8d6f

    mentioned in commit c12c8d6f

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading