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.

Sign up for GitHub

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

Jump to bottom

fix(DataTable): fix cell size and border in FF #2072

Merged
merged 1 commit into from
Oct 17, 2024
Merged

fix(DataTable): fix cell size and border in FF #2072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions src/components/DataTable/DataTable.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,27 @@
}

.data-table__table {
height: fit-content;
table-layout: fixed;
width: 100%;

/* add class instead of tag for styles? */
th,
td {
padding: 0;
vertical-align: top;
/**
* inherit height from 1px size of table rows. This allows height: 100% on cells containers
* to affect full height of cell
*/
height: inherit;
}

.data-table__caption + &,
.data-table__subcaption + & {
margin-top: calc(var(--eds-size-4) / 16 * 1rem);
}
}

.data-table__cell-container, .data-table__header-cell-container {
padding: 0;
vertical-align: top;

&:last-child {
.data-table__cell, .data-table__header-cell {
border-width: 0;
}
}
}

.data-table__search {
width: calc(var(--eds-size-34) / 16 * 1rem);
}
Expand Down Expand Up @@ -193,8 +193,7 @@
*/

.data-table__row {
/* setting the height to 1px so that later, table cell containers can take up 100% of height */
height: 1px;
height: 100%;

&.data-table__row--is-selected {
background-color: var(--eds-theme-color-background-table-row-selected);
Expand Down
2 changes: 1 addition & 1 deletion src/components/DataTable/DataTable.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ export const DefaultWithCustomTable: StoryObj<Args> = {
<table>
<tbody className="border-2 border-utility-default-lowEmphasis-hover">
<tr>
<td>TODO: Custom table rows/cells here</td>
<td>Custom or standard table rows/cells here</td>
</tr>
</tbody>
</table>
Expand Down
11 changes: 9 additions & 2 deletions src/components/DataTable/DataTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ export function DataTable<T>({
<DataTableRow key={headerGroup.id}>
{headerGroup.headers.map((header) => (
<th
className={styles['data-table__header-cell-container']}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a complaint, but curious about this naming format that mizes underscore and dash

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep yep, doc.s here

In our scheme, - has no special meaning and is in place of using camelCase in CSS naming, preferring snake-case. -- and __ have more strict meaning, with most of the rest is convention

HOLD ME ACCOUNTABLE to that page :)

colSpan={header.colSpan}
key={header.id}
style={{
Expand Down Expand Up @@ -243,7 +244,10 @@ export function DataTable<T>({
{row.getLeafRows().map((row) => (
<DataTableRow key={row.id}>
{row.getVisibleCells().map((cell) => (
<td key={cell.id}>
<td
className={styles['data-table__cell-container']}
key={cell.id}
>
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
Expand All @@ -259,7 +263,10 @@ export function DataTable<T>({
isSelected={row.getIsSelected()}
>
{row.getVisibleCells().map((cell) => (
<td key={cell.id}>
<td
className={styles['data-table__cell-container']}
key={cell.id}
>
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
Expand Down
Loading
Loading