- 
                Notifications
    You must be signed in to change notification settings 
- Fork 12
[table] configurable columns #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 
 | 
…long as the string contains a quote that would have to be escaped otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome job!
a few things I noticed:
        
          
                src/components/Table/Table.tsx
              
                Outdated
          
        
      | sortDir?: SortDir; | ||
| sortPosition?: HorizontalDirection; | ||
| width?: string; | ||
| mandatory?: boolean; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP:
requried? a more common term, I think
or maybe configurable: false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed here
| /> | ||
| ))} | ||
| {actionsList.length > 0 && <col width={(actionsList.length + 1) * 32 + 10} />} | ||
| {enableColumnVisibility && <col width={48} />} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| {enableColumnVisibility && ( | ||
| <StyledHeader | ||
| aria-label="Column visibility" | ||
| $size={size} | ||
| > | ||
| <ColumnVisibilityPopover | ||
| headers={headers} | ||
| visibleColumns={visibleColumns} | ||
| onVisibilityChange={onVisibilityChange} | ||
| /> | ||
| </StyledHeader> | ||
| )} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| </ActionsContainer> | ||
| </ActionsList> | ||
| )} | ||
| {enableColumnVisibility && <TableData $size={size} />} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this?
| <EllipsisContent component="div">{label}</EllipsisContent> | ||
| </TableData> | ||
| ))} | ||
| {visibleItems.map(({ label, ...cellProps }, visibleIndex) => { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like on mobile we don't have a way to configure visible columns
we need to keep the button available somewhere
        
          
                src/components/Table/Table.tsx
              
                Outdated
          
        
      |  | ||
| headers.forEach((header, index) => { | ||
| const columnId = header.id || `column-${index}`; | ||
| // If mandatory, always visible. Otherwise, check stored preference (default true) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have a way to mark columns as initially hidden?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed here
        
          
                src/components/Table/Table.tsx
              
                Outdated
          
        
      | > | ||
| <Checkbox | ||
| checked={isVisible} | ||
| disabled={isMandatory} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed here
        
          
                src/components/Table/Table.tsx
              
                Outdated
          
        
      | <ColumnVisibilityHeader> | ||
| <Text | ||
| size="sm" | ||
| weight="semibold" | ||
| > | ||
| Columns | ||
| </Text> | ||
| </ColumnVisibilityHeader> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't think this heading serves any purpose, lets not add it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed here
| display: flex; | ||
| align-items: center; | ||
| gap: 0.5rem; | ||
| cursor: ${({ $disabled }) => ($disabled ? "not-allowed" : "pointer")}; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual interactive area doesn't match the visually interactive area.
You can click at the empty space in the row, and it'll work. But usualy a checkbox clickable area is limited to its label, where there's an actual text.
It's a good thing that the row is clickable, but let's add visual feedback to make it obvious: a hover effect for the whole row, sometimng similar to what we have in CheckboxMultiSelect, or Dropdown
| size?: TableSize; | ||
| showHeader?: boolean; | ||
| rowHeight?: string; | ||
| tableId?: string; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when not specified, it will silently not do anything
we need to make it required when the primary prop is enabled
type Type = 
  | {
    enabled: true;
    tableId: string;
  }
  | {
    enabled?: false;
    tableId?: never;
  };        
          
                src/components/Table/Table.tsx
              
                Outdated
          
        
      | <Popover.Trigger> | ||
| <IconButton | ||
| type="ghost" | ||
| icon="settings" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think I'm more used to seeing a gear icon in this context
but idk, this one works as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed here
…s the non required column), rename mandatory -> required, use lock icon for required col



Add configurable column visibility to Table component
This PR adds the ability for users to show/hide optional columns in the Table component, with preferences automatically persisted to storage.
New Features
mandatory: trueto prevent them from being hiddenNew Props
TableHeaderType:
id?: string- Unique identifier for the column (auto-generated if not provided)required?: boolean- If true, column cannot be hidden by the userselected?: boolean- alternative torequired: if true, the non-required column will be rendered as optional and selectedTable:
enableColumnVisibility?: boolean- Enables the column visibility feature (default: false)tableId?: string- Unique identifier for storing preferences (required when enableColumnVisibility is true)onLoadColumnVisibility?: (tableId: string) => Record<string, boolean>- Optional custom function to load preferences (defaults to localStorage)onSaveColumnVisibility?: (tableId: string, visibility: Record<string, boolean>) => void- Optional custom function to save preferences (defaults to localStorage)Usage
By default, preferences are saved to localStorage with key click-ui-table-column-visibility-{tableId}. Optionally provide custom storage functions for alternative storage
mechanisms (API, IndexedDB, sessionStorage, etc.).
Backward Compatibility
✅ Fully backward compatible - existing tables continue to work unchanged. The feature is opt-in via enableColumnVisibility prop.
click-ui-table-column-visibility.webm