From 0784719a218c87e20fabc042ae4885d012d15931 Mon Sep 17 00:00:00 2001 From: dananji Date: Tue, 5 Dec 2023 16:53:48 -0500 Subject: [PATCH 1/5] Implement reading Canvas-level metadata for MetadataDisplay component --- .../MetadataDisplay/MetadataDisplay.js | 52 +++++++++++-- .../MetadataDisplay/MetadataDisplay.md | 1 + .../MetadataDisplay/MetadataDisplay.test.js | 71 +++++++++++------ src/components/Transcript/Transcript.js | 3 +- src/services/iiif-parser.js | 61 +++++++++++---- src/services/iiif-parser.test.js | 78 +++++++++++++------ src/test_data/playlist.js | 12 +++ 7 files changed, 207 insertions(+), 71 deletions(-) diff --git a/src/components/MetadataDisplay/MetadataDisplay.js b/src/components/MetadataDisplay/MetadataDisplay.js index 4a35c711..f15dee24 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.js +++ b/src/components/MetadataDisplay/MetadataDisplay.js @@ -1,24 +1,61 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useManifestState } from '../../context/manifest-context'; -import { parseMetadata } from '@Services/iiif-parser'; +import { getMetadata } from '@Services/iiif-parser'; import './MetadataDisplay.scss'; -const MetadataDisplay = ({ displayTitle = true, showHeading = true }) => { - const { manifest } = useManifestState(); +/** + * @param {Boolean} param0 read canvas-level metadata when set to true, defaults to false + * @param {Boolean} param1 hide the title in the metadata when set to false, defaults to true + * @param {Boolean} param2 hide the heading UI component when set to false, defaults to true + * @returns + */ +const MetadataDisplay = ({ readCanvasMetadata = false, displayTitle = true, showHeading = true }) => { + const { manifest, canvasIndex } = useManifestState(); const [metadata, setMetadata] = React.useState(); + const [canvasMetadata, setCanvasMetadata] = React.useState(); + /** + * On the initialization of the component read metadata from the Manifest + * or Canvases based on the input props and set the initial set of metadata + * in the component's state + */ React.useEffect(() => { if (manifest) { - let parsedMetadata = parseMetadata(manifest); + let initMetadata = []; + const parsedMetadata = getMetadata(manifest, readCanvasMetadata); + if (readCanvasMetadata) { + setCanvasMetadata(parsedMetadata.canvasMetadata); + initMetadata = parsedMetadata + .canvasMetadata + .filter((m) => m.canvasindex === canvasIndex)[0].metadata; + } else { + initMetadata = parsedMetadata.manifestMetadata; + } if (!displayTitle) { - parsedMetadata = parsedMetadata.filter(md => md.label.toLowerCase() != 'title'); + initMetadata = initMetadata.filter(md => md.label.toLowerCase() != 'title'); } - setMetadata(parsedMetadata); + setMetadata(initMetadata); } }, [manifest]); + /** + * When displaying Canvas-level metadata in the component, update the metadata + * in the component's state listening to the canvasIndex changes in the central + * state + */ + React.useEffect(() => { + if (canvasIndex >= 0 && readCanvasMetadata && canvasMetadata != undefined) { + let currentMetadata = canvasMetadata + .filter((m) => m.canvasindex === canvasIndex)[0].metadata; + if (!displayTitle) { + currentMetadata = currentMetadata.filter(md => md.label.toLowerCase() != 'title'); + } + setMetadata(currentMetadata); + } + }, [canvasIndex]); + if (metadata && metadata.length > 0) { return (
{ return (
-

No valid Metadata is in the Manifest

+

No valid Metadata is in the Manifest/Canvas(es)

); } }; MetadataDisplay.propTypes = { + readCanvasMetadata: PropTypes.bool, displayTitle: PropTypes.bool, showHeading: PropTypes.bool, }; diff --git a/src/components/MetadataDisplay/MetadataDisplay.md b/src/components/MetadataDisplay/MetadataDisplay.md index 5c9cfb76..058e4aca 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.md +++ b/src/components/MetadataDisplay/MetadataDisplay.md @@ -3,6 +3,7 @@ MetadataDisplay component, renders any available metadata in a given IIIF manife `MetadataDisplay` component allows the following props; - `displayTitle`: accepts a Boolean value, which has a default value of `true` and is _not required_. This allows to hide the title in the `MetadataDisplay` component if it's included in the metadata of the IIIF manifest. In some use-cases where the title is already visible in some other part of the page, this can be used to avoid displaying the title in multiple places. - `showHeading`: accepts a Boolean value, which has a default value of `true` and is _not required_. This enables to hide the `Details` heading on top of the component allowing to customize the user interface. +- `readCanvasMetadata`: accepts a Boolean value, which has a defualt value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata at Canvas-level instead of Manifest level. To import this component from the library; diff --git a/src/components/MetadataDisplay/MetadataDisplay.test.js b/src/components/MetadataDisplay/MetadataDisplay.test.js index c9610186..b5e765aa 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.test.js +++ b/src/components/MetadataDisplay/MetadataDisplay.test.js @@ -3,55 +3,80 @@ import { render, screen } from '@testing-library/react'; import MetadataDisplay from './MetadataDisplay'; import manifest from '@TestData/lunchroom-manners'; import manfiestWoMetadata from '@TestData/volleyball-for-boys'; +import playlistManifest from '@TestData/playlist'; import { withManifestProvider } from '../../services/testing-helpers'; describe('MetadataDisplay component', () => { describe('with manifest with metadata', () => { - describe('with default prop, displayTitle=true', () => { - beforeEach(() => { + describe('with prop, displayTitle', () => { + test('with default value displays title in metadata', () => { const MetadataDisp = withManifestProvider(MetadataDisplay, { initialState: { manifest }, }); render(); + expect(screen.getByText('Title')).toBeInTheDocument(); }); - test('renders successfully', () => { + test('set to false, doesn\'t display title in metadata', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest }, + displayTitle: false, + }); + render(); expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + expect(screen.queryByText('Title')).not.toBeInTheDocument(); }); - test('displays title in metadata', () => { - expect(screen.getByText('Title')).toBeInTheDocument(); - }); }); - describe('with displayTitle=false', () => { - beforeEach(() => { + describe('with prop, showHeading', () => { + it('with default value displays Details heading', () => { const MetadataDisp = withManifestProvider(MetadataDisplay, { - initialState: { manifest }, - displayTitle: false, + initialState: { manifest } }); render(); - }); - - test('renders successfully', () => { expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); }); - test('doesn\'t display title in metadata', () => { - expect(screen.queryByText('Title')).not.toBeInTheDocument(); + it('set to false doesn\'t display Details heading', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest }, + showHeading: false, + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).not.toBeInTheDocument(); }); }); - it('with showHeading=false doesn\'t display Details heading', () => { - const MetadataDisp = withManifestProvider(MetadataDisplay, { - initialState: { manifest }, - showHeading: false, + describe('with prop, readCanvasMetadata', () => { + it('with default value displays Manifest-level metadata', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest } + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + expect(screen.queryByText('Title')).toBeInTheDocument(); + expect(screen.queryByText('Playlist Manifest [Playlist]')).toBeInTheDocument(); + }); + + it('set to true displays Canvas-level metadata', () => { + let originalError = console.error; + console.error = jest.fn(); + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest, canvasIndex: 0 }, + readCanvasMetadata: true + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + expect(screen.queryByText('Title')).toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).toBeInTheDocument(); + console.error = originalError; }); - render(); - expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); - expect(screen.queryByTestId('metadata-display-title')).not.toBeInTheDocument(); }); }); @@ -64,7 +89,7 @@ describe('MetadataDisplay component', () => { }); render(); expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); - expect(screen.getByText('No valid Metadata is in the Manifest')).toBeInTheDocument(); + expect(screen.getByText('No valid Metadata is in the Manifest/Canvas(es)')).toBeInTheDocument(); expect(console.error).toBeCalledTimes(1); console.error = originalError; }); diff --git a/src/components/Transcript/Transcript.js b/src/components/Transcript/Transcript.js index 60b3ba98..c8558043 100644 --- a/src/components/Transcript/Transcript.js +++ b/src/components/Transcript/Transcript.js @@ -17,7 +17,8 @@ const INVALID_URL_MSG = 'Invalid URL for transcript, please check again.'; /** * * @param {String} param0 ID of the HTML element for the player on page - * @param {Object} param1 transcripts resource + * @param {String} param1 manifest URL to read transcripts from + * @param {Object} param2 transcripts resource * @returns */ const Transcript = ({ playerID, manifestUrl, transcripts = [] }) => { diff --git a/src/services/iiif-parser.js b/src/services/iiif-parser.js index 027eb8cb..81cbd069 100644 --- a/src/services/iiif-parser.js +++ b/src/services/iiif-parser.js @@ -428,33 +428,60 @@ export function getSupplementingFiles(manifest) { /** * @param {Object} manifest + * @param {Boolean} readCanvasMetadata read metadata from Canvas level * @return {Array} list of key value pairs for each metadata item in the manifest */ -export function parseMetadata(manifest) { +export function getMetadata(manifest, readCanvasMetadata) { try { - const metadata = parseManifest(manifest).getMetadata(); - let parsedMetadata = []; - if (metadata?.length > 0) { - metadata.map(md => { - // get value and replace /n characters with
to display new lines in UI - let value = md.getValue().replace(/\n/g, "
"); - let sanitizedValue = sanitizeHtml(value, { ...HTML_SANITIZE_CONFIG }); - parsedMetadata.push({ - label: md.getLabel(), - value: sanitizedValue + let canvasMetadata = []; + let allMetadata = { canvasMetadata: canvasMetadata, manifestMetadata: [] }; + const manifestMetadata = parseManifest(manifest).getMetadata(); + if (readCanvasMetadata) { + let canvases = parseSequences(manifest)[0].getCanvases(); + for (const i in canvases) { + let canvasindex = parseInt(i); + canvasMetadata.push({ + canvasindex: canvasindex, + metadata: parseMetadata( + canvases[canvasindex].getMetadata(), 'Canvas' + ) }); - }); - return parsedMetadata; - } else { - console.error('iiif-parser -> parseMetadata() -> no metadata in Manifest'); - return parsedMetadata; + }; + allMetadata.canvasMetadata = canvasMetadata; } + allMetadata.manifestMetadata = parseMetadata(manifestMetadata, 'Manifest'); + return allMetadata; } catch (e) { - console.error('iiif-parser -> parseMetadata() -> cannot parse manifest, ', e); + console.error('iiif-parser -> getMetadata() -> cannot parse manifest, ', e); throw new Error(GENERIC_ERROR_MESSAGE); } } +/** + * Parse metadata in the Manifest/Canvas into an array of key value pairs + * @param {Array} metadata list of metadata in Manifest + * @param {String} resourceType resource type which the metadata belongs to + * @returns {Array} an array with key value pairs for the metadata + */ +export function parseMetadata(metadata, resourceType) { + let parsedMetadata = []; + if (metadata?.length > 0) { + metadata.map(md => { + // get value and replace /n characters with
to display new lines in UI + let value = md.getValue().replace(/\n/g, "
"); + let sanitizedValue = sanitizeHtml(value, { ...HTML_SANITIZE_CONFIG }); + parsedMetadata.push({ + label: md.getLabel(), + value: sanitizedValue + }); + }); + return parsedMetadata; + } else { + console.error('iiif-parser -> parseMetadata() -> no metadata in ', resourceType); + return parsedMetadata; + } +} + /** * Parse manifest to see if auto-advance behavior present at manifest level * @param {Object} manifest diff --git a/src/services/iiif-parser.test.js b/src/services/iiif-parser.test.js index 318c622d..7598235a 100644 --- a/src/services/iiif-parser.test.js +++ b/src/services/iiif-parser.test.js @@ -79,8 +79,8 @@ describe('iiif-parser', () => { }); test('returns default value when manifest is invalid', () => { // Mock console.error function - let originalError = console.error; - console.error = jest.fn(); + let originalLogger = console.log; + console.log = jest.fn(); expect(iiifParser.getCanvasIndex( { '@context': [ @@ -97,7 +97,7 @@ describe('iiif-parser', () => { ) ).toEqual(0); // Cleanup mock - console.error = originalError; + console.log = originalLogger; }); }); @@ -350,50 +350,82 @@ describe('iiif-parser', () => { }); }); - describe('parseMetadata()', () => { - it('manifest with metadata property returns a list of key, value pairs', () => { - const metadata = iiifParser.parseMetadata(lunchroomManifest); - expect(metadata.length).toBeGreaterThan(0); - expect(metadata[0]).toEqual({ label: "Title", value: "This is the title of the item!" }); - }); - - it('manifest without metadata property returns []', () => { + describe('getMetadata()', () => { + let originalError; + beforeAll(() => { // Mock console.error function - let originalError = console.error; + originalError = console.error; console.error = jest.fn(); - const metadata = iiifParser.parseMetadata(volleyballManifest); - expect(metadata).toEqual([]); - expect(console.error).toBeCalledTimes(1); + }); + afterAll(() => { + // Clen up mock console.error = originalError; }); + describe('reading only manifest-level metadata', () => { + it('manifest with metadata returns a list of key, value pairs', () => { + const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(lunchroomManifest, false); + expect(manifestMetadata.length).toBeGreaterThan(0); + expect(canvasMetadata.length).toEqual(0); + expect(manifestMetadata[0]).toEqual({ label: "Title", value: "This is the title of the item!" }); + }); + + it('manifest without metadata returns []', () => { + const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(volleyballManifest, false); + expect(manifestMetadata).toEqual([]); + expect(canvasMetadata.length).toEqual(0); + expect(console.error).toBeCalledTimes(1); + }); + }); + + describe('reading only canvas-level metadata', () => { + it('canvas with metadata returns a list of key, value pairs', () => { + const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(playlistManifest, true); + expect(manifestMetadata.length).toBeGreaterThan(0); + expect(canvasMetadata.length).toEqual(3); + expect(canvasMetadata[0].metadata[0]).toEqual({ label: "Title", value: "First Playlist Item" }); + // console.error is called twice for the 2 canvases without metadata + expect(console.error).toBeCalledTimes(2); + }); + + + it('canvas without metadata returns []', () => { + const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(playlistManifest, true); + expect(manifestMetadata.length).toBeGreaterThan(0); + expect(canvasMetadata.length).toEqual(3); + expect(canvasMetadata[1].metadata).toEqual([]); + // console.error is called twice for the 2 canvases without metadata + expect(console.error).toBeCalledTimes(2); + }); + }); + it('replaces new line characters with
tags', () => { - const metadata = iiifParser.parseMetadata(lunchroomManifest); - expect(metadata[3]).toEqual({ + const { manifestMetadata, _ } = iiifParser.getMetadata(lunchroomManifest, false); + expect(manifestMetadata[3]).toEqual({ label: "Summary", value: "This is the summary field. It may include a summary of the item.

Does a pre tag exist here?

How about some bold?

Or italics?" }); }); it('sanitize HTML in value for each metadata item', () => { - const metadata = iiifParser.parseMetadata(lunchroomManifest); - expect(metadata[0]).toEqual({ + const { manifestMetadata, _ } = iiifParser.getMetadata(lunchroomManifest); + expect(manifestMetadata[0]).toEqual({ label: "Title", value: "This is the title of the item!" }); - expect(metadata[2]).toEqual({ + expect(manifestMetadata[2]).toEqual({ label: "Main contributors", value: "John Doe
The Avalon Media System Team" }); - expect(metadata[5]).toEqual({ + expect(manifestMetadata[5]).toEqual({ label: "Collection", value: "Testing" }); - expect(metadata[6]).toEqual({ + expect(manifestMetadata[6]).toEqual({ label: "Related Items", value: "IU
Avalon Website" }); - expect(metadata[7]).toEqual({ + expect(manifestMetadata[7]).toEqual({ label: "Notes", value: "" }); }); diff --git a/src/test_data/playlist.js b/src/test_data/playlist.js index a2286975..3dfcd699 100644 --- a/src/test_data/playlist.js +++ b/src/test_data/playlist.js @@ -57,6 +57,18 @@ export default { }, ], annotations: [], + metadata: [ + { + label: { en: ["Title"] }, + value: { none: ["First Playlist Item"] } + }, { + label: { en: ["Date"] }, + value: { none: ["2023"] } + }, { + label: { en: ["Main Contributor"] }, + value: { none: ["Coronet Films"] } + } + ], }, { id: 'http://example.com/manifests/playlist/canvas/2', From 906cf303d021f7d35e8a7aac913c434634adc55c Mon Sep 17 00:00:00 2001 From: dananji Date: Wed, 6 Dec 2023 13:57:08 -0500 Subject: [PATCH 2/5] Add prop to MetadataDisplay comp. to enable displaying both Canvas and Manifest metadata --- .../MetadataDisplay/MetadataDisplay.js | 158 ++++++++++++------ .../MetadataDisplay/MetadataDisplay.md | 5 +- .../MetadataDisplay/MetadataDisplay.scss | 10 +- .../MetadataDisplay/MetadataDisplay.test.js | 76 +++++++-- src/services/iiif-parser.js | 2 +- src/services/iiif-parser.test.js | 18 +- 6 files changed, 191 insertions(+), 78 deletions(-) diff --git a/src/components/MetadataDisplay/MetadataDisplay.js b/src/components/MetadataDisplay/MetadataDisplay.js index f15dee24..f1453a86 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.js +++ b/src/components/MetadataDisplay/MetadataDisplay.js @@ -5,91 +5,147 @@ import { getMetadata } from '@Services/iiif-parser'; import './MetadataDisplay.scss'; /** - * @param {Boolean} param0 read canvas-level metadata when set to true, defaults to false - * @param {Boolean} param1 hide the title in the metadata when set to false, defaults to true - * @param {Boolean} param2 hide the heading UI component when set to false, defaults to true + * @param {Boolean} param0 display only Canvas metadata when set to true with other props are default + * @param {Boolean} param1 display both Manifest and Canvas metadata when set to true + * @param {Boolean} param2 hide the title in the metadata when set to false, defaults to true + * @param {Boolean} param3 hide the heading UI component when set to false, defaults to true * @returns */ -const MetadataDisplay = ({ readCanvasMetadata = false, displayTitle = true, showHeading = true }) => { +const MetadataDisplay = ({ + displayOnlyCanvasMetadata = false, + displayAllMetadata = false, + displayTitle = true, + showHeading = true +}) => { const { manifest, canvasIndex } = useManifestState(); - const [metadata, setMetadata] = React.useState(); + const [manifestMetadata, setManifestMetadata] = React.useState(); + // Metadata for all Canavases in state + const [canvasesMetadata, setCanvasesMetadata] = React.useState(); + // Current Canvas metadata in state const [canvasMetadata, setCanvasMetadata] = React.useState(); + // Boolean flags set according to user props to hide/show metadata + const [showManifestMetadata, setShowManifestMetadata] = React.useState(); + const [showCanvasMetadata, setShowCanvasMetadata] = React.useState(); /** * On the initialization of the component read metadata from the Manifest - * or Canvases based on the input props and set the initial set of metadata - * in the component's state + * and/or Canvases based on the input props and set the initial set(s) of + * metadata in the component's state */ React.useEffect(() => { if (manifest) { - let initMetadata = []; - const parsedMetadata = getMetadata(manifest, readCanvasMetadata); - if (readCanvasMetadata) { - setCanvasMetadata(parsedMetadata.canvasMetadata); - initMetadata = parsedMetadata + // Display Canvas metadata only when specified in the props + const showCanvas = displayOnlyCanvasMetadata || displayAllMetadata; + setShowCanvasMetadata(showCanvas); + const showManifest = !displayOnlyCanvasMetadata || displayAllMetadata; + setShowManifestMetadata(showManifest); + + // Parse metadata from Manifest + const parsedMetadata = getMetadata(manifest, showCanvas); + + // Set Manifest and Canvas metadata in the state variables according to props + if (showCanvas) { + setCanvasesMetadata(parsedMetadata.canvasMetadata); + const canvasData = parsedMetadata .canvasMetadata .filter((m) => m.canvasindex === canvasIndex)[0].metadata; - } else { - initMetadata = parsedMetadata.manifestMetadata; + setCanvasMetadata(canvasData); } - if (!displayTitle) { - initMetadata = initMetadata.filter(md => md.label.toLowerCase() != 'title'); + if (showManifest) { + let manifestMeta = parsedMetadata.manifestMetadata; + if (!displayTitle) { + manifestMeta = manifestMeta.filter(md => md.label.toLowerCase() != 'title'); + } + setManifestMetadata(manifestMeta); } - setMetadata(initMetadata); } }, [manifest]); /** - * When displaying Canvas-level metadata in the component, update the metadata + * When displaying current Canvas's metadata in the component, update the metadata * in the component's state listening to the canvasIndex changes in the central * state */ React.useEffect(() => { - if (canvasIndex >= 0 && readCanvasMetadata && canvasMetadata != undefined) { - let currentMetadata = canvasMetadata + if (canvasesMetadata == undefined) { + return; + } + if (canvasIndex >= 0 && showCanvasMetadata) { + let currentMetadata = canvasesMetadata .filter((m) => m.canvasindex === canvasIndex)[0].metadata; - if (!displayTitle) { + if (!displayTitle && displayOnlyCanvasMetadata) { currentMetadata = currentMetadata.filter(md => md.label.toLowerCase() != 'title'); } - setMetadata(currentMetadata); + setCanvasMetadata(currentMetadata); } }, [canvasIndex]); - if (metadata && metadata.length > 0) { - return ( -
- {showHeading && ( -
-

Details

-
- )} -
- {metadata.map((md, i) => { - return ( - -
{md.label}
-
- - ); - }) - } -
-
- ); - } else { - return (
{ + return canvasMetadata?.length > 0 || manifestMetadata?.length > 0; + }; + + return ( +
-

No valid Metadata is in the Manifest/Canvas(es)

-
); - } + {showHeading && ( +
+

Details

+
+ )} + {hasMetadata() && ( +
+ {showManifestMetadata && manifestMetadata?.length > 0 && ( + + {displayAllMetadata &&

Manifest Details

} + {manifestMetadata.map((md, index) => { + return ( + +
{md.label}
+
+ + ); + })} + + )} + {showCanvasMetadata && canvasMetadata?.length > 0 && ( + + {displayAllMetadata &&

Canvas Details

} + {canvasMetadata.map((md, index) => { + return ( + +
{md.label}
+
+ + ); + })} + + )} +
+ ) + } + { + !hasMetadata() && ( +
+

No valid Metadata is in the Manifest/Canvas(es)

+
+ ) + } +
+ + ); }; MetadataDisplay.propTypes = { - readCanvasMetadata: PropTypes.bool, + displayOnlyCanvasMetadata: PropTypes.bool, + displayAllMetadata: PropTypes.bool, displayTitle: PropTypes.bool, showHeading: PropTypes.bool, }; diff --git a/src/components/MetadataDisplay/MetadataDisplay.md b/src/components/MetadataDisplay/MetadataDisplay.md index 058e4aca..702ca413 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.md +++ b/src/components/MetadataDisplay/MetadataDisplay.md @@ -1,9 +1,10 @@ -MetadataDisplay component, renders any available metadata in a given IIIF manifest. This component reads manifest data from central state management provided by Contexts. Thus it should be wrapped by context providers using `IIIFPlayer` which is the component in Ramp providing these out of the box. +MetadataDisplay component, renders any available metadata in a given IIIF manifest. By default it displays metadata relevant to the Manifest, and can be customized to show Canvas level metadata using the following props. This component reads manifest data from central state management provided by Contexts. Thus it should be wrapped by context providers using `IIIFPlayer` which is the component in Ramp providing these out of the box. `MetadataDisplay` component allows the following props; - `displayTitle`: accepts a Boolean value, which has a default value of `true` and is _not required_. This allows to hide the title in the `MetadataDisplay` component if it's included in the metadata of the IIIF manifest. In some use-cases where the title is already visible in some other part of the page, this can be used to avoid displaying the title in multiple places. - `showHeading`: accepts a Boolean value, which has a default value of `true` and is _not required_. This enables to hide the `Details` heading on top of the component allowing to customize the user interface. -- `readCanvasMetadata`: accepts a Boolean value, which has a defualt value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata at Canvas-level instead of Manifest level. +- `displayOnlyCanvasMetadata`: accepts a Boolean value, which has a default value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata for the current Canvas instead of Manifest. +- `displayAllMetadata`: accepts a Boolean value, which has a default value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata relevant for both current Canvas and Manifest. To import this component from the library; diff --git a/src/components/MetadataDisplay/MetadataDisplay.scss b/src/components/MetadataDisplay/MetadataDisplay.scss index cfe0fb00..217da0a7 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.scss +++ b/src/components/MetadataDisplay/MetadataDisplay.scss @@ -23,11 +23,19 @@ } .ramp--metadata-display-content { - padding: 0 0 1.5rem 1.5rem; + padding: 0 1.5rem 1.5rem; color: $primaryDarker; max-height: 30rem; overflow-y: auto; + p { + font-weight: normal; + padding: 0.5rem 0; + margin: 00 0 0.75rem; + color: $primaryDarker; + border-bottom: 0.1rem solid $primaryDark; + } + dt { font-weight: bold; } diff --git a/src/components/MetadataDisplay/MetadataDisplay.test.js b/src/components/MetadataDisplay/MetadataDisplay.test.js index b5e765aa..22c4924c 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.test.js +++ b/src/components/MetadataDisplay/MetadataDisplay.test.js @@ -7,6 +7,17 @@ import playlistManifest from '@TestData/playlist'; import { withManifestProvider } from '../../services/testing-helpers'; describe('MetadataDisplay component', () => { + let originalLogger; + beforeAll(() => { + // Mock console.log function + originalLogger = console.log; + console.log = jest.fn(); + }); + afterAll(() => { + // Clen up mock + console.log = originalLogger; + }); + describe('with manifest with metadata', () => { describe('with prop, displayTitle', () => { test('with default value displays title in metadata', () => { @@ -51,46 +62,83 @@ describe('MetadataDisplay component', () => { }); }); - describe('with prop, readCanvasMetadata', () => { - it('with default value displays Manifest-level metadata', () => { + describe('with prop, displayOnlyCanvasMetadata', () => { + it('with default value displays Manifest metadata', () => { const MetadataDisp = withManifestProvider(MetadataDisplay, { initialState: { manifest: playlistManifest } }); render(); expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); - expect(screen.queryByText('Title')).toBeInTheDocument(); + + // Has one title field with Manifest metadata + expect(screen.queryAllByText('Title')).toHaveLength(1); expect(screen.queryByText('Playlist Manifest [Playlist]')).toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).not.toBeInTheDocument(); }); - it('set to true displays Canvas-level metadata', () => { - let originalError = console.error; - console.error = jest.fn(); + it('set to true displays only Canvas metadata', () => { const MetadataDisp = withManifestProvider(MetadataDisplay, { initialState: { manifest: playlistManifest, canvasIndex: 0 }, - readCanvasMetadata: true + displayOnlyCanvasMetadata: true }); render(); expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); - expect(screen.queryByText('Title')).toBeInTheDocument(); + + // Has one title field with Manifest metadata + expect(screen.queryAllByText('Title')).toHaveLength(1); + expect(screen.queryByText('Playlist Manifest [Playlist]')).not.toBeInTheDocument(); expect(screen.queryByText('First Playlist Item')).toBeInTheDocument(); - console.error = originalError; + + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); + }); + }); + + describe('with prop, displayAllMetadata', () => { + it('with default value displays only Manifest metadata', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest } + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + + // Has one title field with Manifest metadata + expect(screen.queryAllByText('Title')).toHaveLength(1); + expect(screen.queryByText('Playlist Manifest [Playlist]')).toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).not.toBeInTheDocument(); + }); + + it('set to true displays both Manifest and Canvas metadata', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest, canvasIndex: 0 }, + displayAllMetadata: true + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + + // Has two title fields with both Manifest and Canvas metadata + expect(screen.queryAllByText('Title')).toHaveLength(2); + expect(screen.queryByText('Playlist Manifest [Playlist]')).toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).toBeInTheDocument(); + + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); }); }); }); it('with manifest without metadata renders a message', () => { - // Mock console.error - let originalError = console.error; - console.error = jest.fn(); const MetadataDisp = withManifestProvider(MetadataDisplay, { initialState: { manifest: manfiestWoMetadata }, }); render(); expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-message')).toBeInTheDocument(); expect(screen.getByText('No valid Metadata is in the Manifest/Canvas(es)')).toBeInTheDocument(); - expect(console.error).toBeCalledTimes(1); - console.error = originalError; + expect(console.log).toBeCalledTimes(1); }); }); diff --git a/src/services/iiif-parser.js b/src/services/iiif-parser.js index 81cbd069..c5f9c2c6 100644 --- a/src/services/iiif-parser.js +++ b/src/services/iiif-parser.js @@ -477,7 +477,7 @@ export function parseMetadata(metadata, resourceType) { }); return parsedMetadata; } else { - console.error('iiif-parser -> parseMetadata() -> no metadata in ', resourceType); + console.log('iiif-parser -> parseMetadata() -> no metadata in ', resourceType); return parsedMetadata; } } diff --git a/src/services/iiif-parser.test.js b/src/services/iiif-parser.test.js index 7598235a..9e8cc9f4 100644 --- a/src/services/iiif-parser.test.js +++ b/src/services/iiif-parser.test.js @@ -351,15 +351,15 @@ describe('iiif-parser', () => { }); describe('getMetadata()', () => { - let originalError; + let originalLogger; beforeAll(() => { // Mock console.error function - originalError = console.error; - console.error = jest.fn(); + originalLogger = console.log; + console.log = jest.fn(); }); afterAll(() => { // Clen up mock - console.error = originalError; + console.log = originalLogger; }); describe('reading only manifest-level metadata', () => { @@ -374,7 +374,7 @@ describe('iiif-parser', () => { const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(volleyballManifest, false); expect(manifestMetadata).toEqual([]); expect(canvasMetadata.length).toEqual(0); - expect(console.error).toBeCalledTimes(1); + expect(console.log).toBeCalledTimes(1); }); }); @@ -384,8 +384,8 @@ describe('iiif-parser', () => { expect(manifestMetadata.length).toBeGreaterThan(0); expect(canvasMetadata.length).toEqual(3); expect(canvasMetadata[0].metadata[0]).toEqual({ label: "Title", value: "First Playlist Item" }); - // console.error is called twice for the 2 canvases without metadata - expect(console.error).toBeCalledTimes(2); + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); }); @@ -394,8 +394,8 @@ describe('iiif-parser', () => { expect(manifestMetadata.length).toBeGreaterThan(0); expect(canvasMetadata.length).toEqual(3); expect(canvasMetadata[1].metadata).toEqual([]); - // console.error is called twice for the 2 canvases without metadata - expect(console.error).toBeCalledTimes(2); + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); }); }); From c08f9c960fbc87fa3043988ed8ea7275c4448cf4 Mon Sep 17 00:00:00 2001 From: dananji Date: Wed, 6 Dec 2023 14:28:45 -0500 Subject: [PATCH 3/5] Fix usage of displayTitle prop with new props --- .../MetadataDisplay/MetadataDisplay.js | 33 ++++++++------- .../MetadataDisplay/MetadataDisplay.test.js | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/components/MetadataDisplay/MetadataDisplay.js b/src/components/MetadataDisplay/MetadataDisplay.js index f1453a86..34614d63 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.js +++ b/src/components/MetadataDisplay/MetadataDisplay.js @@ -21,13 +21,18 @@ const MetadataDisplay = ({ const [manifestMetadata, setManifestMetadata] = React.useState(); // Metadata for all Canavases in state - const [canvasesMetadata, setCanvasesMetadata] = React.useState(); + const [canvasesMetadata, _setCanvasesMetadata] = React.useState(); // Current Canvas metadata in state const [canvasMetadata, setCanvasMetadata] = React.useState(); // Boolean flags set according to user props to hide/show metadata const [showManifestMetadata, setShowManifestMetadata] = React.useState(); const [showCanvasMetadata, setShowCanvasMetadata] = React.useState(); + let canvasesMetadataRef = React.useRef(); + const setCanvasesMetadata = (m) => { + _setCanvasesMetadata(m); + canvasesMetadataRef.current = m; + }; /** * On the initialization of the component read metadata from the Manifest * and/or Canvases based on the input props and set the initial set(s) of @@ -47,10 +52,7 @@ const MetadataDisplay = ({ // Set Manifest and Canvas metadata in the state variables according to props if (showCanvas) { setCanvasesMetadata(parsedMetadata.canvasMetadata); - const canvasData = parsedMetadata - .canvasMetadata - .filter((m) => m.canvasindex === canvasIndex)[0].metadata; - setCanvasMetadata(canvasData); + setCanvasMetadataInState(); } if (showManifest) { let manifestMeta = parsedMetadata.manifestMetadata; @@ -68,19 +70,22 @@ const MetadataDisplay = ({ * state */ React.useEffect(() => { - if (canvasesMetadata == undefined) { - return; - } if (canvasIndex >= 0 && showCanvasMetadata) { - let currentMetadata = canvasesMetadata - .filter((m) => m.canvasindex === canvasIndex)[0].metadata; - if (!displayTitle && displayOnlyCanvasMetadata) { - currentMetadata = currentMetadata.filter(md => md.label.toLowerCase() != 'title'); - } - setCanvasMetadata(currentMetadata); + setCanvasMetadataInState(); } }, [canvasIndex]); + /** + * Set canvas metadata in state + */ + const setCanvasMetadataInState = () => { + let canvasData = canvasesMetadataRef.current + .filter((m) => m.canvasindex === canvasIndex)[0].metadata; + if (!displayTitle && displayOnlyCanvasMetadata) { + canvasData = canvasData.filter(md => md.label.toLowerCase() != 'title'); + } + setCanvasMetadata(canvasData); + }; /** * Distinguish whether there is any metadata to be displayed * @returns {Boolean} diff --git a/src/components/MetadataDisplay/MetadataDisplay.test.js b/src/components/MetadataDisplay/MetadataDisplay.test.js index 22c4924c..e882c533 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.test.js +++ b/src/components/MetadataDisplay/MetadataDisplay.test.js @@ -94,6 +94,28 @@ describe('MetadataDisplay component', () => { // console.log is called twice for the 2 canvases without metadata expect(console.log).toBeCalledTimes(2); }); + + it('set to true with displayTitle set to false displays Canvas metadata w/o title', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest, canvasIndex: 0 }, + displayOnlyCanvasMetadata: true, + displayTitle: false + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + + // Doesn't display title + expect(screen.queryByText('Title')).not.toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).not.toBeInTheDocument(); + + // Displays other metadata + expect(screen.queryByText('Date')).toBeInTheDocument(); + expect(screen.queryByText('2023')).toBeInTheDocument(); + + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); + }); }); describe('with prop, displayAllMetadata', () => { @@ -128,6 +150,25 @@ describe('MetadataDisplay component', () => { // console.log is called twice for the 2 canvases without metadata expect(console.log).toBeCalledTimes(2); }); + + it('set to true with displayTitle set to false displays only title in Canvas metadata', () => { + const MetadataDisp = withManifestProvider(MetadataDisplay, { + initialState: { manifest: playlistManifest, canvasIndex: 0 }, + displayAllMetadata: true, + displayTitle: false + }); + render(); + expect(screen.queryByTestId('metadata-display')).toBeInTheDocument(); + expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); + + // Has one title fields for only Canvas metadata + expect(screen.queryAllByText('Title')).toHaveLength(1); + expect(screen.queryByText('Playlist Manifest [Playlist]')).not.toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).toBeInTheDocument(); + + // console.log is called twice for the 2 canvases without metadata + expect(console.log).toBeCalledTimes(2); + }); }); }); From 13f77f50c6d1be5ee8043ba2f3567dd02b842d90 Mon Sep 17 00:00:00 2001 From: Dananji Withana Date: Thu, 14 Dec 2023 11:16:58 -0800 Subject: [PATCH 4/5] Update src/services/iiif-parser.test.js Co-authored-by: Chris Colvard --- src/services/iiif-parser.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/iiif-parser.test.js b/src/services/iiif-parser.test.js index 9e8cc9f4..5426c76c 100644 --- a/src/services/iiif-parser.test.js +++ b/src/services/iiif-parser.test.js @@ -378,7 +378,7 @@ describe('iiif-parser', () => { }); }); - describe('reading only canvas-level metadata', () => { + describe('reading canvas-level metadata', () => { it('canvas with metadata returns a list of key, value pairs', () => { const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(playlistManifest, true); expect(manifestMetadata.length).toBeGreaterThan(0); From c2f8d0d2ff13ca2c13cdf270cd68a269513cdaf5 Mon Sep 17 00:00:00 2001 From: dananji Date: Thu, 14 Dec 2023 11:42:59 -0800 Subject: [PATCH 5/5] Changes from feedback in code-review --- src/components/MetadataDisplay/MetadataDisplay.js | 12 ++++++++---- src/components/MetadataDisplay/MetadataDisplay.md | 4 +++- .../MetadataDisplay/MetadataDisplay.test.js | 6 +++--- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/components/MetadataDisplay/MetadataDisplay.js b/src/components/MetadataDisplay/MetadataDisplay.js index 34614d63..8c4329b5 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.js +++ b/src/components/MetadataDisplay/MetadataDisplay.js @@ -15,7 +15,9 @@ const MetadataDisplay = ({ displayOnlyCanvasMetadata = false, displayAllMetadata = false, displayTitle = true, - showHeading = true + showHeading = true, + itemHeading = 'Item Details', + sectionHeaading = 'Section Details', }) => { const { manifest, canvasIndex } = useManifestState(); @@ -81,7 +83,7 @@ const MetadataDisplay = ({ const setCanvasMetadataInState = () => { let canvasData = canvasesMetadataRef.current .filter((m) => m.canvasindex === canvasIndex)[0].metadata; - if (!displayTitle && displayOnlyCanvasMetadata) { + if (!displayTitle) { canvasData = canvasData.filter(md => md.label.toLowerCase() != 'title'); } setCanvasMetadata(canvasData); @@ -107,7 +109,7 @@ const MetadataDisplay = ({
{showManifestMetadata && manifestMetadata?.length > 0 && ( - {displayAllMetadata &&

Manifest Details

} + {displayAllMetadata &&

{itemHeading}

} {manifestMetadata.map((md, index) => { return ( @@ -120,7 +122,7 @@ const MetadataDisplay = ({ )} {showCanvasMetadata && canvasMetadata?.length > 0 && ( - {displayAllMetadata &&

Canvas Details

} + {displayAllMetadata &&

{sectionHeaading}

} {canvasMetadata.map((md, index) => { return ( @@ -153,6 +155,8 @@ MetadataDisplay.propTypes = { displayAllMetadata: PropTypes.bool, displayTitle: PropTypes.bool, showHeading: PropTypes.bool, + itemHeading: PropTypes.string, + sectionHeaading: PropTypes.string, }; export default MetadataDisplay; diff --git a/src/components/MetadataDisplay/MetadataDisplay.md b/src/components/MetadataDisplay/MetadataDisplay.md index 702ca413..c3ae24a6 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.md +++ b/src/components/MetadataDisplay/MetadataDisplay.md @@ -1,10 +1,12 @@ -MetadataDisplay component, renders any available metadata in a given IIIF manifest. By default it displays metadata relevant to the Manifest, and can be customized to show Canvas level metadata using the following props. This component reads manifest data from central state management provided by Contexts. Thus it should be wrapped by context providers using `IIIFPlayer` which is the component in Ramp providing these out of the box. +MetadataDisplay component, renders any available metadata in a given IIIF manifest. By default it displays metadata relevant to the Manifest, and can be customized to show Canvas level metadata using the following props. Any changes to `displayTitle` prop is applied to both Manifest and Canvas metadata. This component reads manifest data from central state management provided by Contexts. Thus it should be wrapped by context providers using `IIIFPlayer` which is the component in Ramp providing these out of the box. `MetadataDisplay` component allows the following props; - `displayTitle`: accepts a Boolean value, which has a default value of `true` and is _not required_. This allows to hide the title in the `MetadataDisplay` component if it's included in the metadata of the IIIF manifest. In some use-cases where the title is already visible in some other part of the page, this can be used to avoid displaying the title in multiple places. - `showHeading`: accepts a Boolean value, which has a default value of `true` and is _not required_. This enables to hide the `Details` heading on top of the component allowing to customize the user interface. - `displayOnlyCanvasMetadata`: accepts a Boolean value, which has a default value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata for the current Canvas instead of Manifest. - `displayAllMetadata`: accepts a Boolean value, which has a default value of `false` and is _not required_. Setting this to `true` indicates Ramp to read and display metadata relevant for both current Canvas and Manifest. +- `itemHeading`: accepts a String value, which has a default value of '`Item Details`' and is _not required_. This allows to customize the title for the Manifest level metadata list in the component. +- `sectionHeading`: accepts a String value, which has a default value of '`Section Details`' and is _not required_. This allows to customize the title for the Canvas level metadata list in the component To import this component from the library; diff --git a/src/components/MetadataDisplay/MetadataDisplay.test.js b/src/components/MetadataDisplay/MetadataDisplay.test.js index e882c533..339c252b 100644 --- a/src/components/MetadataDisplay/MetadataDisplay.test.js +++ b/src/components/MetadataDisplay/MetadataDisplay.test.js @@ -151,7 +151,7 @@ describe('MetadataDisplay component', () => { expect(console.log).toBeCalledTimes(2); }); - it('set to true with displayTitle set to false displays only title in Canvas metadata', () => { + it('set to true with displayTitle set to false hides title in all metadata', () => { const MetadataDisp = withManifestProvider(MetadataDisplay, { initialState: { manifest: playlistManifest, canvasIndex: 0 }, displayAllMetadata: true, @@ -162,9 +162,9 @@ describe('MetadataDisplay component', () => { expect(screen.queryByTestId('metadata-display-title')).toBeInTheDocument(); // Has one title fields for only Canvas metadata - expect(screen.queryAllByText('Title')).toHaveLength(1); + expect(screen.queryAllByText('Title')).toHaveLength(0); expect(screen.queryByText('Playlist Manifest [Playlist]')).not.toBeInTheDocument(); - expect(screen.queryByText('First Playlist Item')).toBeInTheDocument(); + expect(screen.queryByText('First Playlist Item')).not.toBeInTheDocument(); // console.log is called twice for the 2 canvases without metadata expect(console.log).toBeCalledTimes(2);