Skip to content

Commit

Permalink
State management refactor for structure (#643)
Browse files Browse the repository at this point in the history 
* Initial attempt at retaining collapse/expand sections with custom hooks

* Set section heading active class, cleanup, fix warnings

* Fix failing tests and styling for all types of structures
  • Loading branch information
@Dananji
Dananji authored Sep 20, 2024
1 parent 44ea757 commit 6f2d557
Show file tree
Hide file tree
Showing 10 changed files with 479 additions and 312 deletions.
22 changes: 19 additions & 3 deletions src/components/StructuredNavigation/NavUtils/List.js
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
import React from 'react';
import ListItem from './ListItem';
import PropTypes from 'prop-types';
import SectionHeading from './SectionHeading';

const List = (({ items, sectionRef, structureContainerRef }) => {
const List = (({ items, sectionRef, structureContainerRef, isPlaylist }) => {
const collapsibleContent = (
<ul
data-testid="list"
className="ramp--structured-nav__list"
role="presentation"
>
{items.map((item, index) => {
if (item) {
// Render canvas items as SectionHeadings in non-playlist contexts
if (item.isCanvas && !isPlaylist) {
return <SectionHeading
key={`${item.label}-${index}`}
itemIndex={index + 1}
duration={item.duration}
label={item.label}
sectionRef={sectionRef}
itemId={item.id}
isRoot={item.isRoot}
structureContainerRef={structureContainerRef}
hasChildren={item.items?.length > 0}
items={item.items}
/>;
} else {
return <ListItem
{...item}
sectionRef={sectionRef}
Expand All @@ -28,7 +43,8 @@ const List = (({ items, sectionRef, structureContainerRef }) => {
List.propTypes = {
items: PropTypes.array.isRequired,
sectionRef: PropTypes.object.isRequired,
structureContainerRef: PropTypes.object.isRequired
structureContainerRef: PropTypes.object.isRequired,
isPlaylist: PropTypes.bool.isRequired,
};

export default List;
3 changes: 3 additions & 0 deletions src/components/StructuredNavigation/NavUtils/List.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('List component', () => {
items: [items],
sectionRef,
structureContainerRef,
isPlaylist: false,
};
const ListWithManifest = withManifestAndPlayerProvider(List, {
initialManifestState: { playlist: { isPlaylist: false }, canvasIndex: 0 },
Expand Down Expand Up @@ -123,6 +124,7 @@ describe('List component', () => {
items: [items],
sectionRef,
structureContainerRef,
isPlaylist: false,
};
const ListWithManifest = withManifestAndPlayerProvider(List, {
initialManifestState: { playlist: { isPlaylist: false }, canvasIndex: 0 },
Expand Down Expand Up @@ -155,6 +157,7 @@ describe('List component', () => {
items: [playlistItem],
sectionRef,
structureContainerRef,
isPlaylist: true,
};
beforeEach(() => {
const ListWithManifest = withManifestAndPlayerProvider(List, {
Expand Down
147 changes: 47 additions & 100 deletions src/components/StructuredNavigation/NavUtils/ListItem.js
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import React from 'react';
import List from './List';
import PropTypes from 'prop-types';
import { usePlayerDispatch } from '../../../context/player-context';
import { useManifestState } from '../../../context/manifest-context';
import { autoScroll, checkSrcRange, getMediaFragment } from '@Services/utility-helpers';
import { autoScroll } from '@Services/utility-helpers';
import { LockedSVGIcon } from '@Services/svg-icons';
import SectionHeading from './SectionHeading';
import { useActiveStructure } from '@Services/ramp-hooks';

const ListItem = ({
duration,
Expand All @@ -17,63 +15,38 @@ const ListItem = ({
label,
summary,
homepage,
isRoot,
items,
itemIndex,
rangeId,
canvasDuration,
sectionRef,
structureContainerRef
}) => {
const playerDispatch = usePlayerDispatch();
const { canvasIndex, currentNavItem, playlist } = useManifestState();
const { isPlaylist } = playlist;

let itemIdRef = React.useRef();
itemIdRef.current = id;

let itemLabelRef = React.useRef();
itemLabelRef.current = label;
const liRef = React.useRef(null);

let itemSummaryRef = React.useRef();
itemSummaryRef.current = summary;
const { handleClick, isActiveLi, currentNavItem, isPlaylist } = useActiveStructure({
itemId: id, liRef, sectionRef,
isCanvas,
canvasDuration,
});

const subMenu =
items && items.length > 0 ? (
<List items={items} sectionRef={sectionRef}
structureContainerRef={structureContainerRef}
isPlaylist={isPlaylist}
/>
) : null;

const liRef = React.useRef(null);

const handleClick = React.useCallback((e) => {
e.preventDefault();
e.stopPropagation();

const { start, end } = getMediaFragment(itemIdRef.current, canvasDuration);
const inRange = checkSrcRange({ start, end }, { end: canvasDuration });
/*
Only continue the click action if not both start and end times of
the timespan are not outside Canvas' duration
*/
if (inRange) {
playerDispatch({ clickedUrl: itemIdRef.current, type: 'navClick' });
liRef.current.isClicked = true;
if (sectionRef.current) {
sectionRef.current.isClicked = true;
}
}
});

/*
Auto-scroll active structure item into view only when user is not actively
interacting with structured navigation
*/
React.useEffect(() => {
/*
Auto-scroll active structure item into view only when user is not actively
interacting with structured navigation
*/
if (liRef.current && currentNavItem?.id == itemIdRef.current
if (liRef.current && currentNavItem?.id == id
&& liRef.current.isClicked != undefined && !liRef.current.isClicked
&& structureContainerRef.current.isScrolling != undefined && !structureContainerRef.current.isScrolling) {
&& structureContainerRef.current.isScrolling != undefined
&& !structureContainerRef.current.isScrolling) {
autoScroll(liRef.current, structureContainerRef);
}
// Reset isClicked if active structure item is set
Expand All @@ -85,52 +58,34 @@ const ListItem = ({
const renderListItem = () => {
return (
<React.Fragment key={rangeId}>
{/* For playlist views omit the accordion style display of structure for canvas-level items */}
{isCanvas && !isPlaylist
?
<React.Fragment>
<SectionHeading
itemIndex={itemIndex}
canvasIndex={canvasIndex}
duration={duration}
label={label}
sectionRef={sectionRef}
itemId={itemIdRef.current}
isRoot={isRoot}
handleClick={handleClick}
structureContainerRef={structureContainerRef}
/>
</React.Fragment>
:
<React.Fragment>
{isTitle
?
(<span className="ramp--structured-nav__item-title"
role="listitem"
aria-label={itemLabelRef.current}
>
{itemLabelRef.current}
</span>)
: (
<React.Fragment key={id}>
<div className="tracker"></div>
{isClickable ? (
<React.Fragment>
{isEmpty && <LockedSVGIcon />}
<a role="listitem"
href={homepage && homepage != '' ? homepage : itemIdRef.current}
onClick={handleClick}>
{`${itemIndex}. `}{itemLabelRef.current} {duration.length > 0 ? ` (${duration})` : ''}
</a>
</React.Fragment>
) : (
<span role="listitem" aria-label={itemLabelRef.current}>{itemLabelRef.current}</span>
)}
</React.Fragment>
)
}
</React.Fragment>
}
<React.Fragment>
{isTitle
?
(<span className="ramp--structured-nav__item-title"
role="listitem"
aria-label={label}
>
{label}
</span>)
: (
<React.Fragment key={id}>
<div className="tracker"></div>
{isClickable ? (
<React.Fragment>
{isEmpty && <LockedSVGIcon />}
<a role="listitem"
href={homepage && homepage != '' ? homepage : id}
onClick={handleClick}>
{`${itemIndex}. `}{label} {duration.length > 0 ? ` (${duration})` : ''}
</a>
</React.Fragment>
) : (
<span role="listitem" aria-label={label}>{label}</span>
)}
</React.Fragment>
)
}
</React.Fragment>
</React.Fragment>
);
};
Expand All @@ -140,16 +95,9 @@ const ListItem = ({
<li
data-testid="list-item"
ref={liRef}
className={
'ramp--structured-nav__list-item' +
`${(itemIdRef.current != undefined && (currentNavItem?.id === itemIdRef.current)
&& (isPlaylist || !isCanvas) && currentNavItem?.canvasIndex === canvasIndex + 1)
? ' active'
: ''
}`
}
data-label={itemLabelRef.current}
data-summary={itemSummaryRef.current}
className={`ramp--structured-nav__list-item${isActiveLi ? ' active' : ''}`}
data-label={label}
data-summary={summary}
>
{renderListItem()}
{subMenu}
Expand All @@ -170,7 +118,6 @@ ListItem.propTypes = {
label: PropTypes.string.isRequired,
summary: PropTypes.string,
homepage: PropTypes.string,
isRoot: PropTypes.bool,
items: PropTypes.array.isRequired,
itemIndex: PropTypes.number,
rangeId: PropTypes.string.isRequired,
Expand Down
87 changes: 36 additions & 51 deletions src/components/StructuredNavigation/NavUtils/ListItem.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,62 +31,46 @@ describe('ListItem component', () => {
const canvasItem =
{
id: undefined,
duration: '09:32',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1',
duration: '02:00',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1',
isTitle: true,
isCanvas: true,
itemIndex: 1,
isCanvas: false,
itemIndex: undefined,
isClickable: false,
isEmpty: false,
canvasDuration: 0,
label: 'Lunchroom Manners',
label: 'Introduction',
items: [
{
id: undefined,
duration: '02:00',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1',
isTitle: true,
id: 'https://example.com/manifest/lunchroome_manners/canvas/1#t=0.0,45.321',
duration: '00:45',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1-1',
isTitle: false,
isCanvas: false,
itemIndex: 1,
isClickable: true,
isEmpty: false,
label: 'Part I',
items: [],
canvasDuration: 572.034,
sectionRef: sectionRef,
structureContainerRef,
},
{
id: 'https://example.com/manifest/lunchroome_manners/canvas/1#t=60,120.321',
duration: '01:00',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1-2',
isTitle: false,
isCanvas: false,
itemIndex: undefined,
isClickable: false,
itemIndex: 2,
isClickable: true,
isEmpty: false,
canvasDuration: 0,
label: 'Introduction',
items: [
{
id: 'https://example.com/manifest/lunchroome_manners/canvas/1#t=0.0,45.321',
duration: '00:45',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1-1',
isTitle: false,
isCanvas: false,
itemIndex: 1,
isClickable: true,
isEmpty: false,
label: 'Part I',
items: [],
canvasDuration: 572.034,
sectionRef: sectionRef,
structureContainerRef,
},
{
id: 'https://example.com/manifest/lunchroome_manners/canvas/1#t=60,120.321',
duration: '01:00',
rangeId: 'https://example.com/manifest/lunchroom_manners/range/1-1-2',
isTitle: false,
isCanvas: false,
itemIndex: 2,
isClickable: true,
isEmpty: false,
label: 'Part II',
canvasDuration: 572.034,
items: [],
sectionRef: sectionRef,
structureContainerRef,
},
],
label: 'Part II',
canvasDuration: 572.034,
items: [],
sectionRef: sectionRef,
structureContainerRef,
}
},
],
sectionRef: sectionRef,
structureContainerRef,
Expand Down Expand Up @@ -293,9 +277,10 @@ describe('ListItem component', () => {
initialState: { playlist: { isPlaylist: false }, canvasIndex: 0 },
});
render(<ListItemWithManifest />);
expect(screen.queryAllByTestId('list')).toHaveLength(2);
expect(screen.queryAllByTestId('list-item').length).toEqual(4);
expect(screen.queryAllByTestId('list-item')[0]).toHaveTextContent('1. Lunchroom Manners09:32');
expect(screen.queryAllByTestId('list')).toHaveLength(1);
expect(screen.queryAllByTestId('list-item').length).toEqual(3);
expect(screen.queryAllByTestId('list-item')[0]).toHaveAttribute('data-label', 'Introduction');
expect(screen.queryAllByTestId('list-item')[1]).toHaveTextContent('Part I');
});

test('renders the section as link for playlist manifests', () => {
Expand Down Expand Up @@ -350,8 +335,8 @@ describe('ListItem component', () => {
initialState: { playlist: { isPlaylist: false }, canvasIndex: 0 },
});
render(<ListItemWithManifest />);
expect(screen.queryAllByTestId('list')).toHaveLength(2);
expect(screen.queryAllByTestId('list-item').length).toEqual(4);
expect(screen.queryAllByTestId('list')).toHaveLength(1);
expect(screen.queryAllByTestId('list-item').length).toEqual(3);
expect(screen.queryByText('1. Part I (00:45)')).toBeInTheDocument();
expect(screen
.queryByText('1. Part I (00:45)')
Expand Down
Loading

0 comments on commit 6f2d557

Please sign in to comment.