Table: Allow complex content to be displayed in Pharos tables #840
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 6e9c553 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
brentswisher
force-pushed
the
feature/complex-table-data
branch
3 times, most recently
from
ae1ddc5 to
7dd8cae
Compare
brentswisher
force-pushed
the
feature/complex-table-data
branch
2 times, most recently
from
8304989 to
2904267
Compare
brentswisher
force-pushed
the
feature/complex-table-data
branch
from
2904267 to
b5526d3
Compare
brentswisher
changed the title
brentswisher
requested review from
sirrah-tam,
mtorres3,
satya-achanta-venkata,
daneah and
jialin-he
and removed request for
a team
daneah
reviewed
Jan 28, 2025
brentswisher
commented
Jan 28, 2025
daneah
approved these changes
Jan 28, 2025
sirrah-tam
approved these changes
Jan 29, 2025
12 tasks
|
Waiting on #862 to merge this in |
* fix(table): add resizeObserver to set cell height When using slotted content in a table cell, it currently is not possible to set the content to be 100% of the cell. This is because using a slot takes the content our of the normal flow and it doesn't know what height it should use when set to 100%. By adding a resizeObserver to see the height of the cell and set that height manually, the descendant content can set height:100% and it will work as expected. While each cell will be set to it's own height, the nature of table rows means that all cells in a row will display as the height of the tallest cell, regardless of what height they have manually set. * fix(table): account for border when calculating height
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change: (check at least one)
Is this a breaking change? (check one)
Is the: (complete all)
What does this change address?
Closes #808
The current pharos table component only allows for plain text content to be included. This updates the table to be able to take any type of content that may need to be displayed.
How does this change work?
The table component was updates to accept content in two formats, by padding in an array as a
rowDataparameter (like it does now) or by utilizing a slot for the table contents.To accomplish the slotted version, there are three new components:
PharosTableBody- the equivalent of aPharosTableRow- the equivalent of aPharosTableCell- the equivalent of aThese are needed because the HTML spec for a table element does not recognize a
<slot></slot>as a valid child elements. If you try to pass one in, the table renders without the slotted content, and instead displays it after the table.While this somewhat complicates the markup because we cannot use a semantic table, using proper
rolesfor the custom elements renders a table which looks and acts the same as a native one, including for assistive technology users.Additional context
There has been some conversation among the Pharos maintainers about using a CSS grid based layout instead of
display:table-row. That worked for the initial design, but when integrating it into the sticky header, it ran into a problem. To properly display the table like a semantic table required usingdisplay:contents.That causes the content of the header to "not produce a specific box by themselves. They are replaced by their pseudo-box and their child boxes" - this then makes it impossible to use aninteractionObserverfor the sticky header, so I reverted back to not using a CSS grid.