fix: Wrap TableCell with Link for native browser routing #2532
Conversation
|
|
||
| const useStyles = makeStyles((theme) => ({ | ||
| cell: { | ||
| padding: "0 !important", |
There was a problem hiding this comment.
Why do we need !important in these styles?
There was a problem hiding this comment.
Unfortunately the MUI styles will override, and that can't work for the padding (because this needs to fill the entire cell).
There was a problem hiding this comment.
Got it. I wonder if we could use specificity in class naming to get around this. Not a blocker for this PR.
| cursor: "pointer", | ||
|
|
There was a problem hiding this comment.
?: why are we removing this?
There was a problem hiding this comment.
The pointer still exists, it's just provided by the "a" tag now!
| import { combineClasses } from "../../util/combineClasses" | ||
|
|
||
| // TableCellLink wraps a TableCell filling the entirety with a Link. | ||
| // This allows table rows to be clickable with browser-behavior like ctrl+click. |
| background: "none", | ||
| paddingTop: theme.spacing(2), | ||
| paddingBottom: theme.spacing(2), | ||
| textDecoration: "none !important", |
There was a problem hiding this comment.
!important generally is not a good pattern. I wonder if there's a way around this.
There was a problem hiding this comment.
Could we get more specific with a selector?
|
@AbhineetJain it's not possible to wrap an entire row, only particular cells. I wish! |
|
|
||
| const useStyles = makeStyles((theme) => ({ | ||
| cell: { | ||
| "&.MuiTableCell-root:not(:only-child):first-child, &.MuiTableCell-root:not(:only-child):last-child": { |
Tables were previously using an onClick handler which replicated some Link behavior, but not natively through the browser. Fixes #2525.
Tables were previously using an onClick handler which replicated some
Link behavior, but not natively through the browser.
Fixes #2525.