Skip to content

Commit 26187af

Browse files
committed
Address comments from review
1 parent f6741a6 commit 26187af

File tree

3 files changed

+14
-19
lines changed

3 files changed

+14
-19
lines changed

dotcom-rendering/src/components/FetchMoreGalleriesData.importable.tsx

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { css } from '@emotion/react';
21
import { isNonNullable, isObject } from '@guardian/libs';
32
import { useEffect, useState } from 'react';
43
import { decideFormat } from '../lib/articleFormat';
@@ -12,7 +11,6 @@ import { MoreGalleries } from './MoreGalleries';
1211
import { Placeholder } from './Placeholder';
1312

1413
type Props = {
15-
limit: number; // Limit the number of items shown (the api often returns more)
1614
discussionApiUrl: string;
1715
absoluteServerTimes: boolean;
1816
isAdFreeUser: boolean;
@@ -25,10 +23,6 @@ type MoreGalleriesResponse = {
2523
trails: FETrailType[];
2624
};
2725

28-
const minHeight = css`
29-
min-height: 300px;
30-
`;
31-
3226
const getMedia = (galleryCount?: number): MainMedia | undefined => {
3327
if (typeof galleryCount === 'number') {
3428
return { type: 'Gallery', count: galleryCount.toString() };
@@ -91,7 +85,6 @@ const fetchJson = async (ajaxUrl: string): Promise<MoreGalleriesResponse> => {
9185
};
9286

9387
export const FetchMoreGalleriesData = ({
94-
limit,
9588
discussionApiUrl,
9689
absoluteServerTimes,
9790
isAdFreeUser,
@@ -117,10 +110,19 @@ export const FetchMoreGalleriesData = ({
117110
});
118111
}, [ajaxUrl]);
119112

113+
useEffect(() => {
114+
if (!data) return;
115+
addDiscussionIds(
116+
data.trails
117+
.map((trail) => trail.discussion?.discussionId)
118+
.filter(isNonNullable),
119+
);
120+
}, [data]);
121+
120122
if (error) {
121123
// Send the error to Sentry and then prevent the element from rendering
122124
window.guardian.modules.sentry.reportError(error, 'more-galleries');
123-
return undefined;
125+
return null;
124126
}
125127

126128
if (!data?.trails) {
@@ -147,22 +149,16 @@ export const FetchMoreGalleriesData = ({
147149
);
148150
}
149151

150-
addDiscussionIds(
151-
data.trails
152-
.map((trail) => trail.discussion?.discussionId)
153-
.filter(isNonNullable),
154-
);
155-
156152
return (
157153
<div
158154
css={{
159-
backgroundColor: palette('--onward-background'),
160-
minHeight: 300,
155+
backgroundColor: palette('--onward-background'),
156+
minHeight: 300,
161157
}}
162158
>
163159
<MoreGalleries
164160
absoluteServerTimes={absoluteServerTimes}
165-
trails={buildTrails(data.trails, limit, isAdFreeUser)}
161+
trails={buildTrails(data.trails, 5, isAdFreeUser)}
166162
discussionApiUrl={discussionApiUrl}
167163
guardianBaseUrl={guardianBaseUrl}
168164
/>

dotcom-rendering/src/components/MoreGalleries.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ const getDefaultCardProps = (
100100
dataLinkName: trail.dataLinkName,
101101
snapData: trail.snapData,
102102
discussionApiUrl,
103-
discussionId: trail.discussionId,
103+
discussionId: trail.discussion?.discussionId,
104104
avatarUrl: trail.avatarUrl,
105105
mainMedia: trail.mainMedia,
106106
isExternalLink: false,

dotcom-rendering/src/layouts/GalleryLayout.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,6 @@ export const GalleryLayout = (props: WebProps | AppProps) => {
365365
<FetchMoreGalleriesData
366366
ajaxUrl={gallery.frontendData.config.ajaxUrl}
367367
guardianBaseUrl={gallery.frontendData.guardianBaseURL}
368-
limit={5}
369368
discussionApiUrl={discussionApiUrl}
370369
absoluteServerTimes={
371370
switches['absoluteServerTimes'] ?? false

0 commit comments

Comments
 (0)