Skip to content

Commit

Permalink
🐛 Refactor dependencies, select row by name and provider (#1554)
Browse files Browse the repository at this point in the history
Resolves: #1509

The Dependencies table was setup to use the `AnalysisDependency.name` as
the selected row key. Since the `.name` field is not unique across all
rows multiple rows would be erroneously selected at the same time. To
solve the problem, a UI only unique id is generated after fetching and
used for the table selection itemId.

Change details:
  - Dependencies fetch query updated to inject a UI uid and return
    `WithUiId<AnalysisDependency>[]`. This transform is done outside the
    react-query `useQuery()` handling.

  - Updated the table to use the `WithUiId<>` key `_ui_unique_id` for
    selection.

  - Since the button rendered in the "Found in" column had no function,
    the button was removed.

  - "Found in" column moved to the last column to match the placement of
    matching columns on the Archetypes and Issues tables.

  - Use i18next plural aware keys for the "Found in" text to avoid the
    "(s)" suffix.

  - Fixed the `id` and `aria-label` on the table.

  - Added `key` to the label column `Label`s.

Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
  • Loading branch information
sjd78 authored Nov 17, 2023
1 parent f10823a commit 5c3fda2
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 33 deletions.
2 changes: 2 additions & 0 deletions client/public/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@
"selectMany": "Select {{what}}",
"selectOne": "Select a {{what}}",
"selectAn": "Select an {{what}}",
"totalApplications": "{{count}} application",
"totalApplications_plural": "{{count}} applications",
"workPriority": "Work priority (1=low, 10=high)"
},
"dialog": {
Expand Down
54 changes: 23 additions & 31 deletions client/src/app/pages/dependencies/dependencies.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from "react";
import {
Button,
Label,
LabelGroup,
PageSection,
Expand Down Expand Up @@ -30,7 +29,6 @@ import { useFetchDependencies } from "@app/queries/dependencies";
import { useSelectionState } from "@migtools/lib-ui";
import { DependencyAppsDetailDrawer } from "./dependency-apps-detail-drawer";
import { useSharedAffectedApplicationFilterCategories } from "../issues/helpers";
import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
import { getParsedLabel } from "@app/utils/rules-utils";

export const Dependencies: React.FC = () => {
Expand All @@ -47,8 +45,6 @@ export const Dependencies: React.FC = () => {
foundIn: "Found in",
provider: "Language",
labels: "Labels",
sha: "SHA",
version: "Version",
},
isFilterEnabled: true,
isSortEnabled: true,
Expand Down Expand Up @@ -101,7 +97,7 @@ export const Dependencies: React.FC = () => {

const tableControls = useTableControlProps({
...tableControlState, // Includes filterState, sortState and paginationState
idProperty: "name",
idProperty: "_ui_unique_id",
currentPageItems,
totalItemCount,
isLoading: isFetching,
Expand All @@ -123,7 +119,7 @@ export const Dependencies: React.FC = () => {
getTrProps,
getTdProps,
},
activeItemDerivedState: { activeItem, clearActiveItem, setActiveItem },
activeItemDerivedState: { activeItem, clearActiveItem },
} = tableControls;

return (
Expand Down Expand Up @@ -151,14 +147,18 @@ export const Dependencies: React.FC = () => {
</ToolbarItem>
</ToolbarContent>
</Toolbar>
<Table {...tableProps} aria-label="Migration waves table">
<Table
{...tableProps}
id="dependencies-table"
aria-label="Dependencies table"
>
<Thead>
<Tr>
<TableHeaderContentWithControls {...tableControls}>
<Th {...getThProps({ columnKey: "name" })} />
<Th {...getThProps({ columnKey: "foundIn" })} />
<Th {...getThProps({ columnKey: "provider" })} />
<Th {...getThProps({ columnKey: "labels" })} />
<Th {...getThProps({ columnKey: "foundIn" })} />
</TableHeaderContentWithControls>
</Tr>
</Thead>
Expand All @@ -182,36 +182,28 @@ export const Dependencies: React.FC = () => {
<Td width={25} {...getTdProps({ columnKey: "name" })}>
{dependency.name}
</Td>
<Td width={10} {...getTdProps({ columnKey: "foundIn" })}>
<Button
className={spacing.pl_0}
variant="link"
onClick={(_) => {
if (activeItem && activeItem === dependency) {
clearActiveItem();
} else {
setActiveItem(dependency);
}
}}
>
{`${dependency.applications} application(s)`}
</Button>
</Td>
<Td width={10} {...getTdProps({ columnKey: "provider" })}>
{dependency.provider}
</Td>
<Td width={10} {...getTdProps({ columnKey: "labels" })}>
<LabelGroup>
{dependency?.labels?.map((label) => {
if (getParsedLabel(label).labelType !== "language")
return (
<Label>
{getParsedLabel(label).labelValue}
</Label>
);
{dependency?.labels?.map((label, index) => {
const { labelType, labelValue } =
getParsedLabel(label);

return labelType !== "language" ? (
<Label key={`${index}-${labelValue}`}>
{labelValue}
</Label>
) : undefined;
})}
</LabelGroup>
</Td>
<Td width={10} {...getTdProps({ columnKey: "foundIn" })}>
{t("composed.totalApplications", {
count: dependency.applications,
})}
</Td>
</TableRowContentWithControls>
</Tr>
))}
Expand All @@ -227,7 +219,7 @@ export const Dependencies: React.FC = () => {
</PageSection>
<DependencyAppsDetailDrawer
dependency={activeItem || null}
onCloseClick={() => setActiveItem(null)}
onCloseClick={() => clearActiveItem()}
/>
</>
);
Expand Down
26 changes: 24 additions & 2 deletions client/src/app/queries/dependencies.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { useMemo } from "react";
import { useQuery } from "@tanstack/react-query";
import {
AnalysisAppDependency,
AnalysisDependency,
HubPaginatedResult,
HubRequestParams,
WithUiId,
} from "@app/api/models";
import { getAppDependencies, getDependencies } from "@app/api/rest";

export interface IDependenciesFetchState {
result: HubPaginatedResult<AnalysisDependency>;
result: HubPaginatedResult<WithUiId<AnalysisDependency>>;
isFetching: boolean;
fetchError: unknown;
refetch: () => void;
Expand All @@ -32,8 +34,28 @@ export const useFetchDependencies = (
onError: (error) => console.log("error, ", error),
keepPreviousData: true,
});

const result = useMemo(() => {
if (!data) {
return { data: [], total: 0, params };
}

const syntheticData: WithUiId<AnalysisDependency>[] = data.data.map(
(dep) => ({
...dep,
_ui_unique_id: `${dep.name}/${dep.provider}`,
})
);

return {
data: syntheticData,
total: data.total,
params: data.params,
};
}, [data, params]);

return {
result: data || { data: [], total: 0, params },
result,
isFetching: isLoading,
fetchError: error,
refetch,
Expand Down

0 comments on commit 5c3fda2

Please sign in to comment.