-
Notifications
You must be signed in to change notification settings - Fork 99
fix: fix error with hidden content for data table #4046
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.
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
base: release-23.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1689,4 +1689,4 @@ After selecting the maximum possible number of rows, you can display an error me | |
| </DataTable> | ||
| ); | ||
| } | ||
| ``` | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,13 +10,25 @@ interface TableCellProps { | |
| column: { | ||
| /** Class(es) to be applied to the cells in the given column */ | ||
| cellClassName?: string; | ||
| /** Uniq ID of the column */ | ||
| id?: string; | ||
| }; | ||
| } | ||
| function TableCell({ getCellProps, render, column }: TableCellProps) { | ||
| const { className, ...rest } = getCellProps(); | ||
|
|
||
| const isActionColumn = column.id === 'action'; | ||
| const cellClasses = classNames(className, column.cellClassName); | ||
|
|
||
| return ( | ||
| <td {...rest} className={classNames('pgn__data-table-cell-wrap', className, column.cellClassName)}> | ||
| {render('Cell')} | ||
| <td {...rest} className={cellClasses}> | ||
| {!isActionColumn ? ( | ||
| <div className="pgn__data-table-cell-wrap"> | ||
| {render('Cell')} | ||
| </div> | ||
| ) : ( | ||
| render('Cell') | ||
| )} | ||
|
Comment on lines
+25
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic feels odd to me.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brian-smith-tcril sorry for delay! The The |
||
| </td> | ||
| ); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't anything enforcing
actionas the value for the column id, meaning this is adding an invisible requirement for consumers.I think adding an explicit prop for overflow would be a less brittle solution here, but I'm open to other ideas on how best to handle this.