Clean up table head styles #3674
Conversation
* move all selectors to one place * delete unused code * remove unneeded nesting
| ) | ||
|
|
||
| expect( | ||
| headerCell.classList.contains(styles.sortingHeaderCellDesc) |
There was a problem hiding this comment.
The sortingHeaderCellDesc and sortingHeaderCellDesc styles aren't actually being used outside of tests. I deleted the classes and corresponding tests, but if this was intentional for testing purposes, I can add them back :)
There was a problem hiding this comment.
Is the behaviour under test elsewhere (maybe that an arrow/indicator is shown)? If not then should add another test with maybe a data-testid. No reason to keep the class around just for this.
| @@ -9,7 +9,7 @@ interface HeaderProps { | |||
| export const Header: React.FC<HeaderProps> = ({ name }) => { | |||
| return ( | |||
| <OverflowHoverTooltip content={name}> | |||
| <div className={styles.headerCellWrapper}> | |||
| <div className={styles.headerCellText}> | |||
There was a problem hiding this comment.
Maybe it's just me, but headerCellWrapper gives me the impression that the class wraps the cell when in reality, it's inside the cell, wrapping the text. Renamed it to headerCellText.
| [styles.sortingHeaderCellAsc]: sortOrder === SortOrder.ASCENDING, | ||
| [styles.sortingHeaderCellDesc]: | ||
| sortOrder === SortOrder.DESCENDING && !header.isPlaceholder, | ||
| [styles.oneRowHeaderCell]: onlyOneLine |
There was a problem hiding this comment.
menuEnabled, sortingHeaderCellAsc, and sortingHeaderCellDesc weren't used in the styles file so I deleted the classes.
| @@ -104,7 +104,7 @@ export const TableHeaderCellContents: React.FC<{ | |||
| setMenuSuppressed, | |||
| resizerHeight | |||
| }) => { | |||
| const isTimestamp = header.headerGroup.id === ColumnType.TIMESTAMP | |||
| const isTimestamp = header.id === 'Created' | |||
There was a problem hiding this comment.
Funnily enough, there were two attempts in the code to move the timestamps icon menu to the right:
- uses
isTimestampand astyles.moveToRightclass - relies on
createdHeaderCellclass to move the icon menu to the right
The first way wasn't technically working so the second way was the one being applied to the actual cell. I decided to go the first way so I fixed the isTimestamp check and deleted the createdHeaderCell class.
| height: auto; | ||
| background-color: $header-bg-color; | ||
|
|
||
| .dropTarget { |
There was a problem hiding this comment.
.dropTarget also not actually being used anywhere.
| [styles.firstLevelHeaderCell]: isFirstLevelHeader(header.column.id), | ||
| [styles.leafHeaderCell]: header.subHeaders === undefined, | ||
| [styles.oneRowHeaderCell]: onlyOneLine, | ||
| [styles.dropTargetHeaderCell]: headerDropTargetId === header.id |
There was a problem hiding this comment.
Since all of these object classes accomplish the same thing (applying extra styles to headerCell/placeHolderCell), I renamed them to all have the HeaderCell at the end, basing that off the first five.
2/4
main<= #3667 <= this <= #3681 <= #3707Part of #3597