Skip to content
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

Feat/table #2003

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Feat/table #2003

wants to merge 23 commits into from

Conversation

BLuEScioN
Copy link
Collaborator

@BLuEScioN BLuEScioN commented Feb 12, 2025

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Adds the table component for our design system.
This PR will be followed by PRs for the additions to the table component:

  • Pagination
  • Tx table with its unique cell renders
  • Block table with its unique implementation for handling grouping

Why accessor and cellrenderer:

  • accessor is more flexible, allowing complex transformations (like row => ${row.firstName} ${row.lastName})
  • Having both gives you the choice between simple property access and complex data transformations2.
  • id is a unique string identifier for the column
  • columnId is a numeric index, useful for maintaining column order and optimizing rendering - This separation allows for both human-readable identifiers and efficient internal operations.
  • cellRenderer handles the visual presentation of the data - Decouples data transformation (accessor) from presentation logic - Enables custom components, formatting, or interactive elements within cells
  • Why have an accessor vs just using the cellrender?
  • Accessor:
  • Handles data transformation/extraction
  • Returns the raw data value
  • Used for sorting operations
  • Can be reused across different cell renderers
  • Keeps business logic separate from presentation
  • CellRenderer:
  • Handles only the presentation/UI
  • Takes the processed data from accessor and renders it
  • Can include formatting, styling, and interactive elements
  • Focuses purely on how to display the data

Issue ticket number and link

#1995
#1996
#1998

Checklist:

  • I have performed a self-review of my code.
  • I have tested the change on desktop and mobile.
  • I have added thorough tests where applicable.
  • I've added analytics and error logging where applicable.

Screenshots (if appropriate):

Copy link

vercel bot commented Feb 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 17, 2025 1:16am
hiro-explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 17, 2025 1:16am

@andresgalante
Copy link
Member

Thanks for putting this together, Nick!

In my experience, large monolithic components like this table can be tough to maintain and not flexible enough to accommodate future changes. I’d suggest breaking this down into smaller, composable parts for the different variations of filters or rows. That would make it easier to extend and also much simpler to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants