Skip to content

Commit

Permalink
🐛 Fix broken rendering of migration waves table rows, properly pass i…
Browse files Browse the repository at this point in the history 
…sExpandable prop in useTableControlProps (#1398)

In #1345, we changed the structure of the expandable rows in the
Migration Waves table and caused the `isExpanded` prop to be present on
all rows (which should only be on the expanded content rows), which
caused all rows to be hidden because it was false. This PR restores the
intended structure: Each row is wrapped in a `<Tbody>` with the
`isExpanded` prop, which contains two `<Tr>`s: one for the row itself
and one for the expanded content.

Also removes the `cursor: "pointer"` inline style on the `Tbody` which
was causing the whole row to appear clickable even though only the
expandable cells are clickable.

Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
  • Loading branch information
@mturley
mturley committed Sep 25, 2023
1 parent 4ab5428 commit de9b20d
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 173 deletions.
3 changes: 2 additions & 1 deletion client/src/app/hooks/table-controls/useTableControlProps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const useTableControlProps = <
TItem,
TColumnKey extends string,
TSortableColumnKey extends TColumnKey,
TFilterCategoryKey extends string = string
TFilterCategoryKey extends string = string,
>(
args: IUseTableControlPropsArgs<
TItem,
Expand Down Expand Up @@ -102,6 +102,7 @@ export const useTableControlProps = <

const tableProps: Omit<TableProps, "ref"> = {
variant,
isExpandable: !!expandableVariant,
};

const getThProps = ({
Expand Down
340 changes: 168 additions & 172 deletions client/src/app/pages/migration-waves/migration-waves.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,198 +363,194 @@ export const MigrationWaves: React.FC = () => {
}
numRenderedColumns={numRenderedColumns}
>
<Tbody style={{ cursor: "pointer" }}>
{currentPageItems?.map((migrationWave, rowIndex) => (
<>
<Tr
key={migrationWave.id}
isExpanded={isCellExpanded(migrationWave)}
{currentPageItems?.map((migrationWave, rowIndex) => (
<Tbody
key={migrationWave.id}
isExpanded={isCellExpanded(migrationWave)}
>
<Tr>
<TableRowContentWithControls
{...tableControls}
item={migrationWave}
rowIndex={rowIndex}
>
<TableRowContentWithControls
{...tableControls}
item={migrationWave}
rowIndex={rowIndex}
<Td width={25} {...getTdProps({ columnKey: "name" })}>
{migrationWave.name}
</Td>
<Td
width={10}
{...getTdProps({ columnKey: "startDate" })}
>
<Td width={25} {...getTdProps({ columnKey: "name" })}>
{migrationWave.name}
</Td>
<Td
width={10}
{...getTdProps({ columnKey: "startDate" })}
>
{dayjs(migrationWave.startDate)
.tz(currentTimezone)
.format("MM/DD/YYYY")}
</Td>
<Td
width={10}
{...getTdProps({ columnKey: "endDate" })}
>
{dayjs(migrationWave.endDate)
.tz(currentTimezone)
.format("MM/DD/YYYY")}
</Td>
<Td
width={10}
{...getCompoundExpandTdProps({
item: migrationWave,
rowIndex,
columnKey: "applications",
})}
>
{migrationWave?.applications?.length.toString()}
</Td>
<Td
width={10}
{...getCompoundExpandTdProps({
{dayjs(migrationWave.startDate)
.tz(currentTimezone)
.format("MM/DD/YYYY")}
</Td>
<Td
width={10}
{...getTdProps({ columnKey: "endDate" })}
>
{dayjs(migrationWave.endDate)
.tz(currentTimezone)
.format("MM/DD/YYYY")}
</Td>
<Td
width={10}
{...getCompoundExpandTdProps({
item: migrationWave,
rowIndex,
columnKey: "applications",
})}
>
{migrationWave?.applications?.length.toString()}
</Td>
<Td
width={10}
{...getCompoundExpandTdProps({
item: migrationWave,
rowIndex,
columnKey: "stakeholders",
})}
>
{migrationWave.allStakeholders.length}
</Td>
<Td
width={20}
{...((!!migrationWave.applications.length ||
migrationWave.status === "No Issues") &&
getCompoundExpandTdProps({
item: migrationWave,
rowIndex,
columnKey: "stakeholders",
})}
>
{migrationWave.allStakeholders.length}
</Td>
<Td
width={20}
{...((!!migrationWave.applications.length ||
migrationWave.status === "No Issues") &&
getCompoundExpandTdProps({
item: migrationWave,
rowIndex,
columnKey: "status",
}))}
columnKey: "status",
}))}
>
{migrationWave.applications.length
? migrationWave.status
: "N/A"}
</Td>
<Td isActionCell id="row-actions">
<Dropdown
isOpen={isRowDropdownOpen === migrationWave.id}
onSelect={() => setIsRowDropdownOpen(null)}
onOpenChange={(_isOpen) =>
setIsRowDropdownOpen(null)
}
popperProps={{ position: "right" }}
toggle={(
toggleRef: React.Ref<MenuToggleElement>
) => (
<MenuToggle
ref={toggleRef}
aria-label="row actions dropdown toggle"
variant="plain"
onClick={() => {
isRowDropdownOpen
? setIsRowDropdownOpen(null)
: setIsRowDropdownOpen(migrationWave.id);
}}
isExpanded={isRowDropdownOpen === rowIndex}
>
<EllipsisVIcon />
</MenuToggle>
)}
shouldFocusToggleOnSelect
>
{migrationWave.applications.length
? migrationWave.status
: "N/A"}
</Td>
<Td isActionCell id="row-actions">
<Dropdown
isOpen={isRowDropdownOpen === migrationWave.id}
onSelect={() => setIsRowDropdownOpen(null)}
onOpenChange={(_isOpen) =>
setIsRowDropdownOpen(null)
<DropdownItem
key="edit"
component="button"
onClick={() => setWaveModalState(migrationWave)}
>
{t("actions.edit")}
</DropdownItem>
<ConditionalTooltip
key="manage-app"
isTooltipEnabled={applications.length === 0}
content={
"No applications are available for assignment."
}
popperProps={{ position: "right" }}
toggle={(
toggleRef: React.Ref<MenuToggleElement>
) => (
<MenuToggle
ref={toggleRef}
aria-label="row actions dropdown toggle"
variant="plain"
onClick={() => {
isRowDropdownOpen
? setIsRowDropdownOpen(null)
: setIsRowDropdownOpen(migrationWave.id);
}}
isExpanded={isRowDropdownOpen === rowIndex}
>
<EllipsisVIcon />
</MenuToggle>
)}
shouldFocusToggleOnSelect
>
<DropdownItem
key="edit"
component="button"
onClick={() => setWaveModalState(migrationWave)}
>
{t("actions.edit")}
</DropdownItem>
<ConditionalTooltip
key="manage-app"
isTooltipEnabled={applications.length === 0}
content={
"No applications are available for assignment."
}
isAriaDisabled={applications.length === 0}
onClick={() => {
setWaveToManageModalState(migrationWave);
}}
>
<DropdownItem
key="manage-app"
isAriaDisabled={applications.length === 0}
onClick={() => {
setWaveToManageModalState(migrationWave);
}}
>
{t("composed.manage", {
what: t("terms.applications").toLowerCase(),
})}
</DropdownItem>
</ConditionalTooltip>
<ConditionalTooltip
{t("composed.manage", {
what: t("terms.applications").toLowerCase(),
})}
</DropdownItem>
</ConditionalTooltip>
<ConditionalTooltip
key="export-to-issue-manager"
isTooltipEnabled={
migrationWave.applications?.length < 1
}
content={
"There are no applications assigned to this migration wave to export."
}
>
<DropdownItem
key="export-to-issue-manager"
isTooltipEnabled={
isDisabled={
migrationWave.applications?.length < 1
}
content={
"There are no applications assigned to this migration wave to export."
}
>
<DropdownItem
key="export-to-issue-manager"
isDisabled={
migrationWave.applications?.length < 1
}
onClick={() =>
setApplicationsToExport(
migrationWave.fullApplications
)
}
>
{t("terms.exportToIssue")}
</DropdownItem>
</ConditionalTooltip>
<DropdownItem
key="delete"
onClick={() =>
setMigrationWaveToDelete({
id: migrationWave.id,
name: migrationWave.name,
})
setApplicationsToExport(
migrationWave.fullApplications
)
}
>
{t("actions.delete")}
{t("terms.exportToIssue")}
</DropdownItem>
</Dropdown>
</Td>
</TableRowContentWithControls>
</Tr>
{isCellExpanded(migrationWave) ? (
<Tr isExpanded>
<Td
{...getExpandedContentTdProps({
item: migrationWave,
})}
>
<ExpandableRowContent>
{isCellExpanded(migrationWave, "applications") &&
!!migrationWave.applications.length ? (
<WaveApplicationsTable
</ConditionalTooltip>
<DropdownItem
key="delete"
onClick={() =>
setMigrationWaveToDelete({
id: migrationWave.id,
name: migrationWave.name,
})
}
>
{t("actions.delete")}
</DropdownItem>
</Dropdown>
</Td>
</TableRowContentWithControls>
</Tr>
{isCellExpanded(migrationWave) ? (
<Tr isExpanded>
<Td
{...getExpandedContentTdProps({
item: migrationWave,
})}
>
<ExpandableRowContent>
{isCellExpanded(migrationWave, "applications") &&
!!migrationWave.applications.length ? (
<WaveApplicationsTable
migrationWave={migrationWave}
removeApplication={removeApplication}
/>
) : isCellExpanded(migrationWave, "stakeholders") &&
!!migrationWave.allStakeholders.length ? (
<WaveStakeholdersTable
migrationWave={migrationWave}
/>
) : (
isCellExpanded(migrationWave, "status") && (
<WaveStatusTable
migrationWave={migrationWave}
removeApplication={removeApplication}
/>
) : isCellExpanded(
migrationWave,
"stakeholders"
) && !!migrationWave.allStakeholders.length ? (
<WaveStakeholdersTable
migrationWave={migrationWave}
/>
) : (
isCellExpanded(migrationWave, "status") && (
<WaveStatusTable
migrationWave={migrationWave}
removeApplication={removeApplication}
/>
)
)}
</ExpandableRowContent>
</Td>
</Tr>
) : null}
</>
))}
</Tbody>
)
)}
</ExpandableRowContent>
</Td>
</Tr>
) : null}
</Tbody>
))}
</ConditionalTableBody>
</Table>
<SimplePagination
Expand Down

0 comments on commit de9b20d

Please sign in to comment.