feat: Changes from review comments, updated filter count to a badge design, applied refactor suggestions, split select-deselect test cases and added reset test h2oai#1689

Author: dbranley

ui/src/table.test.tsx

@@ -1042,7 +1042,21 @@ describe('Table.tsx', () => {
       expect(emitMock).toHaveBeenCalledTimes(1)
     })
 
-    it('Filter counts - manual select, deselect', () => {
+    it('Filter counts - manual select', () => {
+      const { container, getAllByRole, getByLabelText, queryByTestId } = render(<XTable model={tableProps} />)
+
+      expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
+      fireEvent.click(container.querySelector(filterSelectorName)!)
+      //select 2 options
+      fireEvent.click(getByLabelText('1'))
+      expect(getAllByRole('row')).toHaveLength(1 + headerRow)
+      expect(queryByTestId('filter-count')).toHaveTextContent('1')
+      fireEvent.click(getByLabelText('2'))
+      expect(getAllByRole('row')).toHaveLength(2 + headerRow)
+      expect(queryByTestId('filter-count')).toHaveTextContent('2')
+    })    
+
+    it('Filter counts - manual deselect', () => {
       const { container, getAllByRole, getByLabelText, queryByTestId } = render(<XTable model={tableProps} />)
 
       expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
@@ -1064,7 +1078,18 @@ describe('Table.tsx', () => {
       expect(queryByTestId('filter-count')).toBeNull
     })    
 
-    it('Filter counts - select all, deselect all', () => {
+    it('Filter counts - select all', () => {
+      const { container, getAllByRole, queryByTestId, getByText } = render(<XTable model={tableProps} />)
+
+      expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
+      fireEvent.click(container.querySelector(filterSelectorName)!)
+      
+      fireEvent.click(getByText('Select All'))
+      expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
+      expect(queryByTestId('filter-count')).toHaveTextContent(tableProps.rows!.length.toString())
+    })  
+
+    it('Filter counts - deselect all', () => {
       const { container, getAllByRole, queryByTestId, getByText } = render(<XTable model={tableProps} />)
 
       expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
@@ -1077,7 +1102,7 @@ describe('Table.tsx', () => {
       fireEvent.click(getByText('Deselect All'))
       expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
       expect(queryByTestId('filter-count')).toBeNull
-    })  
+    })
 
     it('Filter counts - more than 9 selections', () => {
 
@@ -1110,6 +1135,25 @@ describe('Table.tsx', () => {
       expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
       expect(queryByTestId('filter-count')).toBeNull
     })     
+
+    it('Filter counts - clear selection with reset', () => {
+      const { container, getAllByRole, getByLabelText, queryByTestId, getByText } = render(<XTable model={{ ...tableProps, resettable: true }} />)
+
+      expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
+      fireEvent.click(container.querySelector(filterSelectorName)!)
+      //select 2 options
+      fireEvent.click(getByLabelText('1'))
+      expect(getAllByRole('row')).toHaveLength(1 + headerRow)
+      expect(queryByTestId('filter-count')).toHaveTextContent('1')
+      fireEvent.click(getByLabelText('2'))
+      expect(getAllByRole('row')).toHaveLength(2 + headerRow)
+      expect(queryByTestId('filter-count')).toHaveTextContent('2')
+
+      //hit reset to clear the 2 options
+      fireEvent.click(getByText('Reset table'))
+      expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow)
+      expect(queryByTestId('filter-count')).toBeNull
+    })
   })
 
 

ui/src/table.tsx

@@ -223,14 +223,13 @@ const
       verticalAlign: 'middle',
     },
   filterCount: {
-    position: 'relative', 
-    top: -5, 
-    padding: '0 2px', 
-    color: cssVar('$red'), 
-    border: '0.5px solid', 
-    borderColor: cssVar('$red'), 
-    marginLeft: 5, 
-    lineHeight: 'normal', 
+      position: 'relative', 
+      top: -5, 
+      padding: 5,
+      marginLeft: 5, 
+      lineHeight: 'normal', 
+      borderRadius: 8,
+      backgroundColor: cssVar('$neutralQuaternary'),
     }    
   }),
   styles: Partial<Fluent.IDetailsListStyles> = {
@@ -391,39 +390,37 @@ const
           minWidth,
           maxWidth,
           onRenderHeader: (props?: Fluent.IDetailsColumnProps) => {
-            if (props) {
-              let sortIcon = undefined
-              if (c.sortable) {
-                const sortIconName = sortCols && sortCols[props.column.key] ? 'Sort'+sortCols[props.column.key] : 'Sort'
-                sortIcon = <Fluent.Icon iconName={sortIconName} 
-                                    className={css.sortingIcon} 
-                                    style={{ visibility: `${sortIconName === 'Sort' ? 'hidden' : 'visible'}`}}
-                                  />
-              }
-              let filterCountField = undefined
-              const filterCount = selectedFilters?.[props.column.key]?.length || 0
-              if (filterCount > 0){
-                filterCountField = 
-                  <span data-test='filter-count' className={css.filterCount}>
-                    {`${filterCount > 9 ? '9+' : filterCount}`}
-                  </span>
-              }
-              return <div style={{ display: 'flex', alignItems: 'center'}}>{props.column.name}
-              {sortIcon ? sortIcon : null}
+            if (!props) return null
+
+            const filterCount = selectedFilters?.[props.column.key]?.length || 0
+            const sortIconName = sortCols && sortCols[props.column.key] ? 'Sort'+sortCols[props.column.key] : 'Sort'
 
-              {c.filterable ? (
+            return (
+              <div style={{ display: 'flex', alignItems: 'center'}}>{props.column.name}
+                {c.sortable && (
+                  <Fluent.Icon iconName={sortIconName} 
+                    className={css.sortingIcon} 
+                    style={{ visibility: `${sortIconName === 'Sort' ? 'hidden' : 'visible'}`}}
+                  />                  
+                )}
+
+                {c.filterable && (
                     <Fluent.Icon iconName='ChevronDown' 
                                 onClick={(ev: React.MouseEvent<HTMLElement>) => {
                                     ev.stopPropagation()
                                     onColumnContextMenu(props.column, ev)
                                 }}
                                 className={css.filterIcon}
                     />
-                ) : null
-              }
-              {filterCountField ? filterCountField : null}
-              </div> 
-            } else return null
+                )}
+                
+                {filterCount > 0 && (
+                  <span data-test='filter-count' className={css.filterCount}>
+                    {`${filterCount > 9 ? '9+' : filterCount}`}
+                  </span>
+                )}
+            </div> 
+            )
           },
           onColumnClick,
           columnActionsMode: Fluent.ColumnActionsMode.clickable,
@@ -577,10 +574,6 @@ const
     }, [m.columns, tableToWaveColumn])
     React.useImperativeHandle(ref, () => ({
       resetSortIcons: () => {
-        setColumns(columns => columns.map(col => {
-          if (col.iconName) col.iconName = undefined
-          return col
-        }))
         setSortCols(null)
       }
     }))