Skip to content

Commit a7e65ba

Browse files
jcoynejcoyne
authored
Merge pull request #4247 from ProjectMirador/update-react-window
Update react-window
2 parents 6cd07e4 + 90be647 commit a7e65ba

8 files changed

Lines changed: 177 additions & 113 deletions

‎__tests__/src/components/CompanionWindowFactory.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ describe('CompanionWindowFactory', () => {
6262
content: 'thumbnailNavigation',
6363
});
6464

65-
expect(screen.getByRole('grid')).toHaveAccessibleName('Thumbnails');
65+
expect(screen.getByLabelText('Thumbnails')).toBeInTheDocument();
6666
});
6767
});
6868

‎__tests__/src/components/ThumbnailCanvasGrouping.test.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import manifestJson from '../../fixtures/version-2/019.json';
88

99
/** create wrapper */
1010
function createWrapper(props) {
11+
const canvasGroupings = new CanvasGroupings(
12+
Utils.parseManifest(manifestJson).getSequences()[0].getCanvases(),
13+
).groupings();
14+
1115
return render(
1216
<ThumbnailCanvasGrouping
1317
index={1}
@@ -19,6 +23,9 @@ function createWrapper(props) {
1923
width: 100,
2024
}}
2125
showThumbnailLabels
26+
canvasGroupings={canvasGroupings}
27+
height={131}
28+
position="far-bottom"
2229
{...props}
2330
/>,
2431
);
@@ -27,15 +34,9 @@ function createWrapper(props) {
2734
describe('ThumbnailCanvasGrouping', () => {
2835
let wrapper;
2936
let setCanvas;
30-
const data = {
31-
canvasGroupings: new CanvasGroupings(Utils.parseManifest(manifestJson)
32-
.getSequences()[0].getCanvases()).groupings(),
33-
height: 131,
34-
position: 'far-bottom',
35-
};
3637
beforeEach(() => {
3738
setCanvas = vi.fn();
38-
wrapper = createWrapper({ data, setCanvas });
39+
wrapper = createWrapper({ setCanvas });
3940
});
4041
const spyCurrentCanvasClass = vi.spyOn(ThumbnailCanvasGrouping.prototype, 'currentCanvasClass');
4142
afterEach(() => {
@@ -50,7 +51,7 @@ describe('ThumbnailCanvasGrouping', () => {
5051
it('when clicked, updates the current canvas', async () => {
5152
wrapper.unmount();
5253
const user = userEvent.setup();
53-
wrapper = createWrapper({ data, index: 0, setCanvas });
54+
wrapper = createWrapper({ index: 0, setCanvas });
5455
await user.click(wrapper.container.querySelector('.mirador-thumbnail-nav-canvas-0')); // eslint-disable-line testing-library/no-node-access
5556
expect(spyCurrentCanvasClass).toHaveBeenCalledWith([0]);
5657
expect(spyCurrentCanvasClass).toHaveReturnedWith('current-canvas-grouping');
@@ -59,7 +60,7 @@ describe('ThumbnailCanvasGrouping', () => {
5960
describe('attributes based off far-bottom position', () => {
6061
it('in button div', () => {
6162
expect(screen.getByRole('button', { name: 'Image 1' })).toHaveStyle({
62-
height: '123px',
63+
height: '119px',
6364
width: 'auto',
6465
});
6566
});
@@ -68,10 +69,7 @@ describe('ThumbnailCanvasGrouping', () => {
6869
beforeEach(() => {
6970
wrapper.unmount();
7071
createWrapper({
71-
data: {
72-
...data,
73-
position: 'far-right',
74-
},
72+
position: 'far-right',
7573
setCanvas,
7674
});
7775
});

‎__tests__/src/components/ThumbnailNavigation.test.js‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ Subject.propTypes = {
3030
fixture: PropTypes.object, // eslint-disable-line react/forbid-prop-types
3131
};
3232

33-
vi.mock(
34-
'react-virtualized-auto-sizer',
35-
() => ({
36-
default: ({ children }) => children({ height: 600, width: 600 }),
37-
}),
38-
);
39-
4033
describe('ThumbnailNavigation', () => {
4134
it('renders the component', async () => {
4235
render(<Subject />);
@@ -49,29 +42,49 @@ describe('ThumbnailNavigation', () => {
4942
expect(screen.getAllByRole('gridcell').length).toEqual(3);
5043
});
5144

45+
it('uses Grid component for horizontal (far-bottom) layout', () => {
46+
render(<Subject position="far-bottom" />);
47+
// Grid component renders with role="grid" inside the Thumbnails wrapper
48+
const thumbnailWrapper = screen.getByLabelText('Thumbnails');
49+
const grid = thumbnailWrapper.querySelector('[role="grid"]');
50+
expect(grid).toBeInTheDocument();
51+
// Grid should have 3 gridcells (all in one row)
52+
expect(screen.getAllByRole('gridcell')).toHaveLength(3);
53+
});
54+
55+
it('uses List component for vertical (far-right) layout', () => {
56+
render(<Subject position="far-right" />);
57+
// List component renders with role="list" inside the Thumbnails wrapper
58+
const thumbnailWrapper = screen.getByLabelText('Thumbnails');
59+
const list = thumbnailWrapper.querySelector('[role="list"]');
60+
expect(list).toBeInTheDocument();
61+
// List should have 3 gridcells (each as a separate item)
62+
expect(screen.getAllByRole('gridcell')).toHaveLength(3);
63+
});
64+
5265
// TODO: Test that we recalculate dimensions when the view changes (resetAfterIndex)
5366
// TODO: Test that the window scrolls when the canvasIndex prop changes (scorllToItem)
5467

5568
it('gives the grid a size', () => {
5669
const { rerender } = render(<Subject />);
57-
expect(screen.getByRole('grid')).toHaveStyle({ height: '150px', width: '100%' });
70+
expect(screen.getByLabelText('Thumbnails')).toHaveStyle({ height: '150px', width: '100%' });
5871

5972
rerender(<Subject position="far-right" />);
60-
expect(screen.getByRole('grid')).toHaveStyle({ height: '100%', minHeight: 0, width: '123px' });
73+
expect(screen.getByLabelText('Thumbnails')).toHaveStyle({ height: '100%', minHeight: 0, width: '127px' });
6174
});
6275

6376
it('roughly doubles the width of the grid in book view', () => {
6477
const { rerender } = render(<Subject position="far-right" />);
65-
expect(screen.getByRole('grid')).toHaveStyle({ width: '123px' });
78+
expect(screen.getByLabelText('Thumbnails')).toHaveStyle({ width: '127px' });
6679

6780
rerender(<Subject position="far-right" view="book" />);
68-
expect(screen.getByRole('grid')).toHaveStyle({ width: '223px' });
81+
expect(screen.getByLabelText('Thumbnails')).toHaveStyle({ width: '227px' });
6982
});
7083

7184
it('calculates the scaled width of each cell', () => {
7285
render(<Subject />);
7386

74-
expect(screen.getAllByRole('gridcell')[0]).toHaveStyle({ width: '74px' });
87+
expect(screen.getAllByRole('gridcell')[0]).toHaveStyle({ width: '111px' });
7588
});
7689

7790
it('calculates the scaled height of each cell when on the right', () => {
@@ -82,7 +95,7 @@ describe('ThumbnailNavigation', () => {
8295
it('keeps a minimum size for each cell', () => {
8396
render(<Subject fixture={zeroWidthFixture} />);
8497

85-
expect(screen.getAllByRole('gridcell')[0]).toHaveStyle({ width: '100px' });
98+
expect(screen.getAllByRole('gridcell')[0]).toHaveStyle({ width: '111px' });
8699
});
87100

88101
it('keeps a minimum size for each cell when on the right', () => {
@@ -106,22 +119,22 @@ describe('ThumbnailNavigation', () => {
106119
it('handles down arrow by advancing the current canvas when the canvas is on the right', () => {
107120
render(<Subject canvasIndex={1} hasNextCanvas position="far-right" setNextCanvas={setNextCanvas} />);
108121

109-
screen.getByRole('grid').focus();
110-
fireEvent.keyDown(screen.getByRole('grid'), { code: 'ArrowDown', key: 'ArrowDown' });
122+
screen.getByLabelText('Thumbnails').focus();
123+
fireEvent.keyDown(screen.getByLabelText('Thumbnails'), { code: 'ArrowDown', key: 'ArrowDown' });
111124
expect(setNextCanvas).toHaveBeenCalled();
112125
});
113126
it('handles left arrow by selecting the previous canvas', () => {
114127
render(<Subject canvasIndex={2} hasPreviousCanvas setPreviousCanvas={setPreviousCanvas} />);
115128

116-
screen.getByRole('grid').focus();
117-
fireEvent.keyDown(screen.getByRole('grid'), { code: 'ArrowLeft', key: 'ArrowLeft' });
129+
screen.getByLabelText('Thumbnails').focus();
130+
fireEvent.keyDown(screen.getByLabelText('Thumbnails'), { code: 'ArrowLeft', key: 'ArrowLeft' });
118131
expect(setPreviousCanvas).toHaveBeenCalled();
119132
});
120133
it('handles up arrow by selecting the previous canvas when the canvas is on the right', () => {
121134
render(<Subject canvasIndex={2} hasPreviousCanvas position="far-right" setPreviousCanvas={setPreviousCanvas} />);
122135

123-
screen.getByRole('grid').focus();
124-
fireEvent.keyDown(screen.getByRole('grid'), { code: 'ArrowUp', key: 'ArrowUp' });
136+
screen.getByLabelText('Thumbnails').focus();
137+
fireEvent.keyDown(screen.getByLabelText('Thumbnails'), { code: 'ArrowUp', key: 'ArrowUp' });
125138
expect(setPreviousCanvas).toHaveBeenCalled();
126139
});
127140
});

‎__tests__/src/components/WorkspaceElasticWindow.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe('WorkspaceElasticWindow', () => {
9797
top: -2500,
9898
});
9999

100-
const el = container.querySelector('[style="position: absolute; user-select: none; width: 20px; height: 20px; right: -10px; bottom: -10px; cursor: se-resize;"]');
100+
const el = container.querySelector('[style="position: absolute; user-select: none; width: 20px; height: 20px; z-index: 1; right: -10px; bottom: -10px; cursor: se-resize;"]');
101101

102102
const oldCoords = {
103103
x: 0,

‎package.json‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,8 @@
6666
"react-mosaic-component2": "^7.0.0",
6767
"react-redux": "^8.0.0 || ^9.0.0",
6868
"react-resize-observer": "^1.1.1",
69-
"react-rnd": "~10.4.0",
70-
"react-virtualized-auto-sizer": "^1.0.2",
71-
"react-window": "^1.8.5",
69+
"react-rnd": "~10.5.3",
70+
"react-window": "^2.2.7",
7271
"redux": "^5.0.0",
7372
"redux-saga": "^1.1.3",
7473
"redux-thunk": "^3.1.0",
@@ -90,7 +89,7 @@
9089
"@mui/system": "^7.x",
9190
"@testing-library/dom": "^10.4.0",
9291
"@testing-library/jest-dom": "^6.1.5",
93-
"@testing-library/react": "^16.0.1",
92+
"@testing-library/react": "^16.3.2",
9493
"@testing-library/user-event": "^14.4.3",
9594
"@vitejs/plugin-react": "^5.2.0",
9695
"@vitest/coverage-v8": "^3.0.5",

‎src/components/ThumbnailCanvasGrouping.jsx‎

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,26 +39,48 @@ export class ThumbnailCanvasGrouping extends PureComponent {
3939
/** */
4040
render() {
4141
const {
42-
index, style, data, currentCanvasId, showThumbnailLabels,
42+
index,
43+
columnIndex,
44+
style,
45+
canvasGroupings,
46+
position,
47+
height,
48+
currentCanvasId,
49+
showThumbnailLabels,
4350
} = this.props;
44-
const {
45-
canvasGroupings, position, height,
46-
} = data;
47-
const currentGroupings = canvasGroupings[index];
48-
const SPACING = 8;
51+
// For Grid (horizontal), use columnIndex; for List (vertical), use index
52+
const itemIndex = columnIndex !== undefined ? columnIndex : index;
53+
const currentGroupings = canvasGroupings[itemIndex];
54+
const SPACING = 12;
55+
56+
// In react-window v2 Grid (horizontal layout), columnWidth is passed as style.height
57+
// For List (vertical layout), rowHeight is passed as style.height
58+
const isHorizontal = position === 'far-bottom';
59+
let calculatedWidth = null;
60+
if (isHorizontal && style.height) {
61+
calculatedWidth = style.height - SPACING;
62+
} else if (Number.isInteger(style.width)) {
63+
calculatedWidth = style.width - SPACING;
64+
}
65+
66+
const isSelected = currentGroupings
67+
.map(canvas => canvas.id)
68+
.includes(currentCanvasId);
69+
4970
return (
5071
<div
5172
style={{
5273
...style,
5374
boxSizing: 'content-box',
54-
height: (Number.isInteger(style.height)) ? style.height - SPACING : null,
55-
left: (Number.isInteger(style.left)) ? style.left + SPACING : null,
56-
top: style.top + SPACING,
57-
width: (Number.isInteger(style.width)) ? style.width - SPACING : null,
75+
height: Number.isInteger(style.height) ? style.height - SPACING : null,
76+
left: Number.isInteger(style.left) ? style.left + SPACING / 2 : null,
77+
padding: SPACING / 2,
78+
top: Number.isInteger(style.top) ? style.top + SPACING / 2 : null,
79+
width: calculatedWidth,
5880
}}
5981
className={ns('thumbnail-nav-container')}
6082
role="gridcell"
61-
aria-colindex={index + 1}
83+
aria-colindex={itemIndex + 1}
6284
>
6385
<StyledCanvas
6486
role="button"
@@ -67,19 +89,25 @@ export class ThumbnailCanvasGrouping extends PureComponent {
6789
onClick={this.setCanvas}
6890
tabIndex={-1}
6991
sx={theme => ({
92+
'&:not(:hover)': {
93+
outline: isSelected
94+
? `2px solid ${theme.palette.primary.main}`
95+
: 0,
96+
...(isSelected && {
97+
outlineOffset: '3px',
98+
}),
99+
},
70100
'&:hover': {
71-
outline: `9px solid ${theme.palette.action.hover}`,
72-
outlineOffset: '-2px',
101+
outline: isSelected
102+
? `2px solid ${theme.palette.primary.main}`
103+
: `2px solid ${theme.palette.action.hover}`,
104+
outlineOffset: isSelected ? '3px' : '-2px',
73105
},
74-
height: (position === 'far-right') ? 'auto' : `${height - SPACING}px`,
75-
outline: currentGroupings.map(canvas => canvas.id).includes(currentCanvasId) ? `2px solid ${theme.palette.primary.main}` : 0,
76-
...(currentGroupings.map(canvas => canvas.id).includes(currentCanvasId) && {
77-
outlineOffset: '3px',
78-
}),
79-
width: (position === 'far-bottom') ? 'auto' : `${style.width}px`,
106+
height: position === 'far-right' ? 'auto' : `${height - SPACING}px`,
107+
width: position === 'far-bottom' ? 'auto' : `${style.width}px`,
80108
})}
81109
className={classNames(
82-
ns(['thumbnail-nav-canvas', `thumbnail-nav-canvas-${index}`, this.currentCanvasClass(currentGroupings.map(canvas => canvas.index))]),
110+
ns(['thumbnail-nav-canvas', `thumbnail-nav-canvas-${itemIndex}`, this.currentCanvasClass( currentGroupings.map(canvas => canvas.index))]),
83111
)}
84112
>
85113
{currentGroupings.map((canvas, i) => (
@@ -98,10 +126,18 @@ export class ThumbnailCanvasGrouping extends PureComponent {
98126
}
99127

100128
ThumbnailCanvasGrouping.propTypes = {
129+
canvasGroupings: PropTypes.array.isRequired, // eslint-disable-line react/forbid-prop-types
130+
columnIndex: PropTypes.number,
101131
currentCanvasId: PropTypes.string.isRequired,
102-
data: PropTypes.object.isRequired,
103-
index: PropTypes.number.isRequired,
132+
height: PropTypes.number.isRequired,
133+
index: PropTypes.number,
134+
position: PropTypes.string.isRequired,
104135
setCanvas: PropTypes.func.isRequired,
105136
showThumbnailLabels: PropTypes.bool.isRequired,
106-
style: PropTypes.object.isRequired,
137+
style: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
138+
};
139+
140+
ThumbnailCanvasGrouping.defaultProps = {
141+
columnIndex: undefined,
142+
index: undefined,
107143
};

0 commit comments

Comments
 (0)