Skip to content

Commit

Permalink
Merge pull request #3959 from nulib/4710-work-video-poster-fix
Browse files Browse the repository at this point in the history
Fix bug where Work image toggle removed for video filesets which have a poster set
  • Loading branch information
adamjarling authored May 13, 2024
2 parents b36353f + 6b14ea2 commit d60488b
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 58 deletions.
19 changes: 10 additions & 9 deletions app/assets/js/components/Work/Fileset/List.test.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import React from "react";
import WorkFilesetList from "@js/components/Work/Fileset/List";
import { screen, waitFor } from "@testing-library/react";
import { mockFileSets } from "@js/mock-data/filesets";
import {
renderWithRouterApollo,
withReactBeautifulDND,
} from "@js/services/testing-helpers";
import { screen, waitFor } from "@testing-library/react";

import React from "react";
import WorkFilesetList from "@js/components/Work/Fileset/List";
import { WorkProvider } from "@js/context/work-context";
import { mockFileSets } from "@js/mock-data/filesets";
import { mockUser } from "@js/components/Auth/auth.gql.mock";
import useIsAuthorized from "@js/hooks/useIsAuthorized";
import { WorkProvider } from "@js/context/work-context";

jest.mock("@js/hooks/useIsAuthorized");
useIsAuthorized.mockReturnValue({
Expand All @@ -25,7 +26,7 @@ describe("WorkFilesetList component", () => {
isReordering: true,
})}
</WorkProvider>,
{}
{},
);
await waitFor(() => {
expect(screen.getByTestId("fileset-draggable-list"));
Expand All @@ -39,7 +40,7 @@ describe("WorkFilesetList component", () => {
fileSets: { access: mockFileSets, auxiliary: [] },
})}
</WorkProvider>,
{}
{},
);
await waitFor(() => {
expect(screen.getByTestId("fileset-list"));
Expand All @@ -53,10 +54,10 @@ describe("WorkFilesetList component", () => {
fileSets: { access: mockFileSets, auxiliary: [] },
})}
</WorkProvider>,
{}
{},
);
await waitFor(() => {
expect(screen.getAllByTestId("fileset-item")).toHaveLength(3);
expect(screen.getAllByTestId("fileset-item")).toHaveLength(4);
});
});
});
30 changes: 11 additions & 19 deletions app/assets/js/components/Work/Fileset/ListItem.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { Button, Tag } from "@nulib/design-system";
import React, { useContext } from "react";
import { useWorkDispatch, useWorkState } from "@js/context/work-context";

import AuthDisplayAuthorized from "@js/components/Auth/DisplayAuthorized";
import { IIIFContext } from "@js/components/IIIF/IIIFProvider";
import { IconPlay } from "@js/components/Icon";
import PropTypes from "prop-types";
import React from "react";
import { Tag } from "@nulib/design-system";
import UIFormField from "@js/components/UI/Form/Field";
import UIFormInput from "@js/components/UI/Form/Input";
import UIFormTextarea from "@js/components/UI/Form/Textarea";
Expand All @@ -19,10 +17,9 @@ function WorkFilesetListItem({
isEditing,
workImageFilesetId,
}) {
const iiifServerUrl = useContext(IIIFContext);
const { id, coreMetadata } = fileSet;
const dispatch = useWorkDispatch();
const {isImage, isMedia, isPDF, isZip } = useFileSet();
const { hasRepresentativeImage, isImage, isMedia, isPDF, isZip } =
useFileSet();
const workContextState = useWorkState();

// Helper for media type file sets
Expand All @@ -46,25 +43,25 @@ function WorkFilesetListItem({
}
};

const placeholderImage = (fileSet) => {
const placeholderImage = (fileSet) => {
if (isMedia(fileSet)) {
return "/images/video-placeholder2.png";
} else if (isPDF(fileSet)) {
} else if (isPDF(fileSet)) {
return "/images/placeholder-pdf.png";
} else if (isZip(fileSet)) {
} else if (isZip(fileSet)) {
return "/images/placeholder-zip.png";
} else {
return "/images/placeholder.png";
}
}
};

const showWorkImageToggle = () => {
if (fileSet.role.id === "A" && workContextState.workTypeId === "AUDIO") {
return false;
} else {
return isImage(fileSet);
return isImage(fileSet) || hasRepresentativeImage(fileSet);
}
}
};

return (
<article className="box" data-testid="fileset-item">
Expand Down Expand Up @@ -120,9 +117,7 @@ function WorkFilesetListItem({
<div className="column is-5 has-text-right is-clearfix">
{!isEditing && (
<>
{(
showWorkImageToggle()
) && (
{showWorkImageToggle() && (
<AuthDisplayAuthorized>
<div className="field">
<input
Expand All @@ -147,9 +142,6 @@ function WorkFilesetListItem({
{fileSet.role.id === "X" && (
<WorkFilesetActionButtonsAuxillary fileSet={fileSet} />
)}
{fileSet.role.id === "S" && (
<WorkFilesetActionButtonsSupplemental fileSet={fileSet} />
)}
</>
)}
</div>
Expand Down
72 changes: 62 additions & 10 deletions app/assets/js/components/Work/Fileset/ListItem.test.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,50 @@
import React from "react";
import { render, screen, waitFor } from "@testing-library/react";
import WorkFilesetListItemImage from "./ListItem";
import { mockFileSets } from "@js/mock-data/filesets";
import {
renderWithReactHookForm,
withReactBeautifulDND,
} from "@js/services/testing-helpers";

import React from "react";
import WorkFilesetList from "./ListItem";
import { WorkProvider } from "@js/context/work-context";
import { mockFileSets } from "@js/mock-data/filesets";
import { mockUser } from "@js/components/Auth/auth.gql.mock";
import useIsAuthorized from "@js/hooks/useIsAuthorized";
import { WorkProvider } from "@js/context/work-context";

jest.mock("@js/hooks/useIsAuthorized");
useIsAuthorized.mockReturnValue({
user: mockUser,
isAuthorized: () => true,
});

// Mock child components
jest.mock("@js/components/Work/Fileset/ActionButtons/Access", () => {
return {
__esModule: true,
default: () => {
return <div>Mocked Access</div>;
},
};
});
jest.mock("@js/components/Work/Fileset/ActionButtons/Auxillary", () => {
return {
__esModule: true,
default: () => {
return <div>Mocked Auxillary</div>;
},
};
});

describe("Fileset component", () => {
describe("when not editing", () => {
function setUpTests(workImageFilesetId) {
return render(
<WorkProvider>
<WorkFilesetListItemImage
<WorkFilesetList
fileSet={mockFileSets[0]}
workImageFilesetId={workImageFilesetId}
/>
</WorkProvider>
</WorkProvider>,
);
}
it("renders the image preview, label and description", async () => {
Expand Down Expand Up @@ -54,6 +73,39 @@ describe("Fileset component", () => {
expect(toggleEl).not.toBeChecked();
});
});

it("renders the Work image toggle if the fileset is a video and has a representative image", async () => {
render(
<WorkProvider>
<WorkFilesetList
fileSet={mockFileSets[3]}
workImageFilesetId={mockFileSets[3].id}
/>
</WorkProvider>,
);

const toggleEl = await screen.findByTestId("work-image-selector");
expect(toggleEl).toBeInTheDocument();
});

it("does not render the image toggle if the fileset is a video and does not have a representative image", async () => {
const fileSet = {
...mockFileSets[3],
representativeImageUrl: null,
};

render(
<WorkProvider>
<WorkFilesetList
fileSet={fileSet}
workImageFilesetId={mockFileSets[3].id}
/>
</WorkProvider>,
);

const toggleEl = screen.queryByTestId("work-image-selector");
expect(toggleEl).not.toBeInTheDocument();
});
});

describe("Audio work type", () => {
Expand All @@ -70,22 +122,22 @@ describe("Fileset component", () => {
it("doesn't render the Work image toggle", () => {
render(
<WorkProvider initialState={initialState}>
<WorkFilesetListItemImage
<WorkFilesetList
fileSet={mockFileSets[0]}
workImageFilesetId={undefined}
/>
</WorkProvider>
</WorkProvider>,
);
expect(
screen.queryByTestId("work-image-selector")
screen.queryByTestId("work-image-selector"),
).not.toBeInTheDocument();
});
});
});

describe("when editing", () => {
beforeEach(() => {
const Wrapped = withReactBeautifulDND(WorkFilesetListItemImage, {
const Wrapped = withReactBeautifulDND(WorkFilesetList, {
fileSet: mockFileSets[0],
index: 0,
isEditing: true,
Expand Down
5 changes: 5 additions & 0 deletions app/assets/js/hooks/useFileSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export default function useFileSet() {
return webVtt;
}

function hasRepresentativeImage(fileSet = {}) {
return !!fileSet.representativeImageUrl;
}

function isEmpty(fileSet = {}) {
return !fileSet || Object.keys(fileSet).length === 0;
}
Expand Down Expand Up @@ -78,6 +82,7 @@ export default function useFileSet() {
altFileFormat,
filterFileSets,
getWebVttString,
hasRepresentativeImage,
isAltFormat,
isEmpty,
isImage,
Expand Down
Loading

0 comments on commit d60488b

Please sign in to comment.