patternfly-react icon indicating copy to clipboard operation
patternfly-react copied to clipboard

Bug - Table - Examples using wrong type for sorts

Open gitdallas opened this issue 2 years ago • 4 comments

Several table examples are using number | null for sortIndex and 'asc' | 'desc' | null for sortDirection while the sortBy object wants undefined instead of null.

Came up in this discussion: https://patternfly.slack.com/archives/C4FM977N0/p1703294961208719?thread_ts=1703237597.944969&cid=C4FM977N0

gitdallas avatar Dec 23 '23 01:12 gitdallas

if i use undefined in const [activeSortIndex, setActiveSortIndex] = React.useState<number | undefined>(undefined); then i get error into this block below:

// Note that we perform the sort as part of the component's render logic and not in onSort.
  // We shouldn't store the list of data in state because we don't want to have to sync that with props.
  let sortedRepositories = repositories;
  if (activeSortIndex !== null) {
    sortedRepositories = repositories.sort((a, b) => {
      const aValue = getSortableRowValues(a)[activeSortIndex];----->Error(Type 'undefined' cannot be used as an index type.)
      const bValue = getSortableRowValues(b)[activeSortIndex];----->Error(Type 'undefined' cannot be used as an index type.)
      if (typeof aValue === 'number') {
        // Numeric sort
        if (activeSortDirection === 'asc') {
          return (aValue as number) - (bValue as number);
        }
        return (bValue as number) - (aValue as number);
      } else {
        // String sort
        if (activeSortDirection === 'asc') {
          return (aValue as string).localeCompare(bValue as string);
        }
        return (bValue as string).localeCompare(aValue as string);
      }
    });
  }

yamalameyooooo avatar Dec 23 '23 11:12 yamalameyooooo

defining index: activeSortIndex ?? undefined and direction: activeSortDirection ?? undefined resolves the error at code block below :

const getSortParams = (columnIndex: number): ThProps['sort'] => ({
    sortBy: {
      index: activeSortIndex ?? undefined,
      direction: activeSortDirection ?? undefined
    },
    onSort: (_event, index, direction) => {
      setActiveSortIndex(index);
      setActiveSortDirection(direction as 'desc' | 'asc');
    },
    columnIndex
  });

yamalameyooooo avatar Dec 23 '23 11:12 yamalameyooooo

That error is because you didn't update the null check to be an undefined check. I prefer using undefined personally.

gitdallas avatar Dec 24 '23 00:12 gitdallas

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Mar 02 '24 18:03 stale[bot]