Skip to content

Fix x-table-cell border-bottom alignment across columns#3

Merged
avanelsas merged 1 commit intomainfrom
fix/x-table-cell-border-alignment
Apr 13, 2026
Merged

Fix x-table-cell border-bottom alignment across columns#3
avanelsas merged 1 commit intomainfrom
fix/x-table-cell-border-alignment

Conversation

@avanelsas
Copy link
Copy Markdown
Owner

@avanelsas avanelsas commented Apr 13, 2026

Summary

Cell border-bottom lines did not connect across columns in x-table / x-table-row when the row mixed cells with different intrinsic heights (e.g. a header cell containing a sort button vs a plain label cell).

Root cause: x-table-cell's inner [part=cell] wrapper — which carries the border-bottom declaration — used display: flex with no height: 100%. When x-table-row's subgrid stretched the host to the row's height via default align-items: stretch, [part=cell] stayed at its content height instead of filling the host. Different cells → different border-bottom y-positions → disconnected row dividers.

Fix: one line — add height: 100% to [part=cell].

Why height: 100% is safe here

  • Grid context (x-table → x-table-row subgrid → x-table-cell): CSS Grid treats align-items: stretch as a definite track-based size, so the percentage resolves correctly and the wrapper fills the row height. Border-bottoms line up.
  • Standalone context (x-table-cell outside any grid/flex parent): the host has no definite height, so height: 100% on a child computes to auto — identical to today's behaviour, no regression.
  • min-height: 100% would work too but is less clear about intent; changing the host's display to flex would be more invasive.

Regression test

Added part-cell-fills-host-height-test in test/baredom/components/x_table_cell/x_table_cell_test.cljs. Mounts the cell inside a fixed-80px container, forces the host to height: 100%, and asserts the shadow [part=cell] offsetHeight equals 80. Catches the regression if anyone removes the new rule.

Test suite: 3633 tests, all passing.

Test plan

  • npm test — 3633 tests pass including the new regression test
  • npx shadow-cljs watch app and open demo/x-table.html — Basic / Sortable / Selectable tables all show continuous horizontal borders between columns, regardless of whether the header row has sort buttons
  • Same check on demo/x-table-row.html and demo/x-table-cell.html
  • Theme smoke-test: append ?theme=ocean / ?theme=forest / ?theme=aurora to the URL and confirm borders still connect across presets
  • Standalone sanity: mount an x-table-cell outside any x-table/x-table-row and confirm it still renders at its natural content height

🤖 Generated with Claude Code

When x-table-row's subgrid stretches the x-table-cell host to the
row's height (via default align-items:stretch), the inner [part=cell]
wrapper stayed at its content height instead of filling the host.
Since border-bottom lives on [part=cell], it ended up at different
y-positions between columns with different intrinsic content heights
(e.g. a header cell with a sort button vs a plain label cell), and
the horizontal row dividers did not connect across the table.

Add "height:100%" on [part=cell] so it fills whatever definite height
the host is given by its grid/flex parent. In the grid case, Grid
treats align-items:stretch as a definite track-based size, so the
percentage resolves correctly. In the standalone case (no
grid/flex parent), percentage height resolves to auto — identical to
today's behaviour, no regression.

Add a regression test that mounts the cell inside a fixed-height
container, forces the host to height:100%, and asserts [part=cell]
offsetHeight equals the container height.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@avanelsas avanelsas merged commit a165415 into main Apr 13, 2026
1 check passed
avanelsas added a commit that referenced this pull request Apr 13, 2026
Patch release. The only shipped library code change since v2.1.0 is
the x-table-cell border-bottom alignment fix from PR #3 — every other
commit on main since v2.1.0 lives under demo/** or public/index.html
(dev-only scaffolding) and has no impact on the published npm /
Clojars artefacts.

Version bumps in lockstep:
- package.json
- build.clj
- deps.edn (artifact path)
- README.md (deps.edn / Leiningen / npm install snippets)

CHANGELOG entry summarises the x-table-cell fix and lists the demo
dogfooding work under a "Demos" section so consumers reading the
changelog can see what made up the 39 commits without overstating
them as user-facing features.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@avanelsas avanelsas mentioned this pull request Apr 13, 2026
5 tasks
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.

1 participant