fix(components): Allow colSpan to be optional in DataTable.Footer #2882
Conversation
Deploying atlantis with
|
| Latest commit: |
4c61b96
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d922447a.atlantis.pages.dev |
| Branch Preview URL: | https://taylor-datatable-footer.atlantis.pages.dev |
|
Published Pre-release for e1926a2 with versions: To install the new version(s) for Web run: |
| * directly, allowing custom row/cell structures. | ||
| */ | ||
| readonly colSpan: number; | ||
| readonly colSpan?: number; |
There was a problem hiding this comment.
Would be nice to see a visual integration test verifying this behaviour! Would prevent future breakage. Not sure if DataTable has visual tests already though, so we might be missing more than just this feature.
There was a problem hiding this comment.
to my knowledge it doesn't have visual integration tests but that is something I can absolutely do in a separate PR, if that's ok with you! I've been meaning to
There was a problem hiding this comment.
Up to whether you have time. Visual tests are very much appreciated and ideally we've been trying to make sure all PRs have them.
Worth emphasizing that this being a change, ideally the visual test is written first to confirm that after the change, the screenshots don't change for existing cases. And then add a new test verifying the new functionality as well.
I'm not holding this PR up on that. Will approve after I'm done testing here 👍
There was a problem hiding this comment.
Understood! Thanks Jacob. I will fast follow
| </DataTable.Footer> | ||
|
|
||
| {/* Totals footer - omits colSpan for column-aligned content */} | ||
| <DataTable.Footer> |
There was a problem hiding this comment.
I see that this Footer is below the pagination Footer above. However, it's rendered in the opposite order? I'm not familiar with this, so I'm curious why that's happening/if it's expected. I briefly checked the Pagination docs and I don't see this Footer order documented, maybe I'm missing something?
Motivations
Updates
DataTable.Footerto support two usage patterns by making thecolSpanprop optional. This enables column-aligned footer content (like totals rows) in addition to the existing full-width footer content (like pagination).Changes
Added
DataTableFooter.tsxcolSpanprop optionalcolSpanis provided: wraps children in a single<td>spanning all columns (existing behavior)colSpanis omitted: renders children directly, allowing custom<Row>/<Cell>structuresDataTableNotes.mdxComposable.stories.tsxWithTotalsFooterstory demonstrating column-aligned totals with pagination spans the entire tableTesting
Visual testing in Storybook:
WithTotalsFooterstory renders with totals under appropriate columns, while paginationChanges can be
tested via Pre-release
In Atlantis we use Github's built in pull request reviews.