Skip to content

Feat/implement mobile tables#644

Open
maryxan wants to merge 13 commits into
mainfrom
feat/implement-mobile-tables
Open

Feat/implement mobile tables#644
maryxan wants to merge 13 commits into
mainfrom
feat/implement-mobile-tables

Conversation

@maryxan

@maryxan maryxan commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds mobile-specific row expansion support to the Table component while preserving the existing desktop table behavior.

On mobile, each logical row is now rendered as a compact summary row with an optional expanded details section. The implementation keeps expansion state internally in the table, exposes that state through the imperative ref API, and adds an onRowExpand callback so consuming applications can react when a row expands or collapses. Cell renderers also now receive expansion context through ctx.isExpanded, which allows client code to adapt mobile rendering based on row state.

The mobile implementation includes dedicated layout/styling for the summary row and expanded details row, keeps row selection independent from expansion, and stabilizes zebra striping by grouping each logical row in its own tbody.

Changes

Mobile row rendering

Added mobile-specific row rendering via a new component:

  • MobileTableRow.tsx

Mobile rows now render as:

  • One summary row
  • One conditional expanded details row

Expansion is handled internally through expandedRows state in the table reducer.

Row expansion logic

  • Added toggleRowExpanded reducer action and wired it through the table body
  • Added onRowExpand(rowId, isExpanded) callback to notify consumers when a row expands/collapses
  • Kept selection and expansion independent

Table API updates

Extended types.ts with:

  • CellContext (isExpanded)
  • hideOnMobile?: boolean on columns
  • onRowExpand?: (rowId, isExpanded) => void
  • renderMobileRightActions?: (row) => ReactNode

Expanded imperative table handlers with:

  • getExpandedRows()
  • isRowExpanded(rowId)

Cell rendering

  • Column/row render functions now receive an optional ctx argument with isExpanded
  • Mobile primary and expanded detail cells receive ctx.isExpanded based on row state
  • Desktop cells continue rendering with isExpanded: false
  • Added renderRowValue(...) helper to centralize row value rendering

Table structure

  • Each logical data row now renders inside its own tbody.table-row-group
  • Replaced the previous single shared tbody

This ensures zebra striping remains stable when expanded rows are inserted.

Header & mobile column behavior

  • Header now marks the default column with primary-header
  • Mobile layout hides all other headers and keeps only the primary header visible

@maryxan
maryxan marked this pull request as draft April 8, 2026 07:36
@maryxan maryxan self-assigned this Apr 8, 2026
@maryxan maryxan added the feature label Apr 8, 2026
@maryxan
maryxan marked this pull request as ready for review April 16, 2026 07:45

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

prefer having one expect(case) peer it(test)

Comment thread src/components/Table/styles.ts Outdated
cursor: pointer;
}

@media screen and (max-width: 767px) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it worth making this the default one and the desktop to be inside a @media

Comment thread src/components/Table/styles.ts Outdated
}

@media screen and (max-width: 767px) {
table {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it worth using proper bem classes

Comment thread src/components/Table/styles.ts Outdated

.mobile-expanded-label {
font-size: 0.875rem;
font-weight: 700;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

don't we have a var for size and weight?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we only have var for font-size

}) => {
const accessors = getVisibleAccessors(columns);
const defaultAccessor = getDefaultAccessor(columns);
const isMobile = windowWidth < breakpoints.md;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be accessed by a hook?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

splitting it in more components it would probably help readability


th,
td {
display: none;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this hides all the data cells expect from the primary one. The styles below display: none; are applied to the primary one

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is still confusing for me this one

}

.autohide-cell {
> div {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

prefer using specific classes

width: auto;
padding-inline-end: 0;

div {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

prefer using specific classes

Comment on lines +351 to +366
> div {
opacity: 1;
}
}
}

.autohide-cell:focus-within {
> div {
opacity: 1;
}
}
}
}

.autohide-cell {
> div {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

prefer using specific classes instead of div

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants