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

refactor: table component #157

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Conversation

deansallinen
Copy link
Contributor

Removes the Table component and reverts to using class-based styles on standard html table elements.

Reason: There was no need for the overhead of a component at this stage, with all the prop passing and data typing, when all we're doing is applying styles.

Once I started using the table component for something more complex than displaying tabular data (i.e. the search history) I found there would be too may edge cases and complexity was ballooning.

I think we're better served with the following:

  • using standard <table> and associated elements e.g. <td> which are all styled when the .table-styles class is applied to the <table> for all non-interactive tabular data
  • if you want to get more complicated e.g. collapsing layouts on mobile, linking rows to pages, etc. then you're better off using a div class="grid" and tailoring the component to the context.

Example: in the search history, the rows needed to be clickable. Using a <a> as a child of <tbody> is not valid. Adding an onclick to the <tr> works, but is not accessible and hard to conditionally style (i.e. showing a pointer cursor only when the row is clickable). So in this case, I used a grid of links styled with the .table-row-styles class to match the appearance of a default table.

This allows for flexibility when needed while keeping simplicity in the base <table> and maintaining consistent styles throughout

@deansallinen deansallinen merged commit c14c4d2 into search-history Oct 7, 2024
4 checks passed
@deansallinen deansallinen deleted the refactor-component-table branch October 7, 2024 18:40
@deansallinen deansallinen linked an issue Oct 8, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table mobile responsiveness
1 participant