Skip to content

Commit 36f0abb

Browse files
scottdickersonscottdickerson
authored
Merge pull request #1886 from carbon-design-system/fix-dashboard-editor
fix(dashboardeditor): need to refactor function to react component so it can be memoized
2 parents caac10f + 00e4626 commit 36f0abb

6 files changed

Lines changed: 400 additions & 339 deletions

File tree

‎src/components/BarChartCard/BarChartCard.jsx‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -87,28 +87,31 @@ const BarChartCard = ({
8787

8888
const memoizedGenerateSampleValues = useMemo(
8989
() =>
90-
generateSampleValues(
91-
series,
92-
timeDataSourceId,
93-
interval,
94-
timeRange,
95-
categoryDataSourceId
96-
),
90+
isEditable
91+
? generateSampleValues(
92+
series,
93+
timeDataSourceId,
94+
interval,
95+
timeRange,
96+
categoryDataSourceId
97+
)
98+
: [],
9799
// eslint-disable-next-line react-hooks/exhaustive-deps
98-
[series, interval, timeRange]
100+
[series, interval, timeRange, isEditable]
99101
);
100102

101103
const memoizedGenerateSampleValuesForEditor = useMemo(
102104
() =>
103-
generateSampleValuesForEditor(
104-
valuesProp,
105-
categoryDataSourceId,
106-
timeDataSourceId,
107-
availableDimensions,
108-
interval,
109-
timeRange
110-
),
111-
// eslint-disable-next-line react-hooks/exhaustive-deps
105+
isDashboardPreview
106+
? generateSampleValuesForEditor(
107+
valuesProp,
108+
categoryDataSourceId,
109+
timeDataSourceId,
110+
availableDimensions,
111+
interval,
112+
timeRange
113+
)
114+
: [],
112115
[
113116
availableDimensions,
114117
categoryDataSourceId,
@@ -117,7 +120,6 @@ const BarChartCard = ({
117120
interval,
118121
timeRange,
119122
valuesProp,
120-
series.length,
121123
]
122124
);
123125

‎src/components/DashboardEditor/DashboardEditor.jsx‎

Lines changed: 145 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useState, useEffect } from 'react';
1+
import React, { useState, useEffect, useMemo, useCallback } from 'react';
22
import PropTypes from 'prop-types';
33
import { InlineNotification, SkeletonText } from 'carbon-components-react';
44
import isNil from 'lodash/isNil';
@@ -17,10 +17,10 @@ import ImageGalleryModal, {
1717
} from '../ImageGalleryModal/ImageGalleryModal';
1818

1919
import DashboardEditorHeader from './DashboardEditorHeader/DashboardEditorHeader';
20+
import DashboardEditorCardRenderer from './DashboardEditorCardRenderer';
2021
import {
2122
getDefaultCard,
2223
getDuplicateCard,
23-
getCardPreview,
2424
renderBreakpointInfo,
2525
handleKeyDown,
2626
handleOnClick,
@@ -257,7 +257,7 @@ const DashboardEditor = ({
257257
isLoading,
258258
i18n,
259259
}) => {
260-
const mergedI18n = { ...defaultProps.i18n, ...i18n };
260+
const mergedI18n = useMemo(() => ({ ...defaultProps.i18n, ...i18n }), [i18n]);
261261
// Need to keep track of whether the image gallery is open or not
262262
const [isImageGalleryModalOpen, setIsImageGalleryModalOpen] = useState(false);
263263

@@ -290,78 +290,88 @@ const DashboardEditor = ({
290290
* Adds a default, empty card to the preview
291291
* @param {string} type card type
292292
*/
293-
const addCard = (type) => {
294-
const cardConfig = getDefaultCard(type, mergedI18n);
295-
setDashboardJson({
296-
...dashboardJson,
297-
cards: [...dashboardJson.cards, cardConfig],
298-
});
299-
setSelectedCardId(cardConfig.id);
300-
};
293+
const addCard = useCallback(
294+
(type) => {
295+
const cardConfig = getDefaultCard(type, mergedI18n);
296+
// eslint-disable-next-line no-shadow
297+
setDashboardJson((dashboardJson) => ({
298+
...dashboardJson,
299+
cards: [...dashboardJson.cards, cardConfig],
300+
}));
301+
setSelectedCardId(cardConfig.id);
302+
},
303+
[mergedI18n]
304+
);
301305

302306
/**
303307
* Adds a cloned card with a new unique id to the preview
304308
* @param {string} id
305309
*/
306-
const duplicateCard = (id) => {
307-
const cardConfig = getDuplicateCard(
308-
dashboardJson.cards.find((i) => i.id === id)
309-
);
310-
setDashboardJson({
311-
...dashboardJson,
312-
cards: [...dashboardJson.cards, cardConfig],
310+
const duplicateCard = useCallback((id) => {
311+
// eslint-disable-next-line no-shadow
312+
setDashboardJson((dashboardJson) => {
313+
const cardConfig = getDuplicateCard(
314+
dashboardJson.cards.find((i) => i.id === id)
315+
);
316+
return {
317+
...dashboardJson,
318+
cards: [...dashboardJson.cards, cardConfig],
319+
};
313320
});
314-
setSelectedCardId(cardConfig.id);
315-
};
321+
setSelectedCardId(id);
322+
}, []);
316323

317324
/**
318325
* Deletes a card from the preview
319326
* @param {string} id
320327
*/
321-
const removeCard = (id) =>
322-
setDashboardJson({
323-
...dashboardJson,
324-
cards: dashboardJson.cards.filter((i) => i.id !== id),
325-
});
326-
327-
const onSelectCard = (id) => setSelectedCardId(id);
328-
const onDuplicateCard = (id) => duplicateCard(id);
329-
const onRemoveCard = (id) => removeCard(id);
328+
const removeCard = useCallback(
329+
(id) =>
330+
// eslint-disable-next-line no-shadow
331+
setDashboardJson((dashboardJson) => ({
332+
...dashboardJson,
333+
cards: dashboardJson.cards.filter((i) => i.id !== id),
334+
})),
335+
[]
336+
);
330337

331-
const handleOnCardChange = (cardConfig) => {
332-
// need to handle resetting the src of the image for image cards based on the id
333-
if (
334-
cardConfig.type === CARD_TYPES.IMAGE &&
335-
cardConfig.content.imgState !== 'new'
336-
) {
337-
// eslint-disable-next-line no-param-reassign
338-
cardConfig.content.src = availableImages.find(
339-
(image) => image.id === cardConfig.content.id
340-
)?.src;
341-
} else if (
342-
cardConfig.content.imgState === 'new' &&
343-
!imagesToUpload.some((image) => image.id === cardConfig.content.id)
344-
) {
345-
if (cardConfig.content.id && cardConfig.content.src) {
346-
setImagesToUpload((prevImagesToUpload) => [
347-
...prevImagesToUpload,
348-
{ id: cardConfig.content.id, src: cardConfig.content.src },
349-
]);
338+
const handleOnCardChange = useCallback(
339+
(cardConfig) => {
340+
// need to handle resetting the src of the image for image cards based on the id
341+
if (
342+
cardConfig.type === CARD_TYPES.IMAGE &&
343+
cardConfig.content.imgState !== 'new'
344+
) {
345+
// eslint-disable-next-line no-param-reassign
346+
cardConfig.content.src = availableImages.find(
347+
(image) => image.id === cardConfig.content.id
348+
)?.src;
349+
} else if (
350+
cardConfig.content.imgState === 'new' &&
351+
!imagesToUpload.some((image) => image.id === cardConfig.content.id)
352+
) {
353+
if (cardConfig.content.id && cardConfig.content.src) {
354+
setImagesToUpload((prevImagesToUpload) => [
355+
...prevImagesToUpload,
356+
{ id: cardConfig.content.id, src: cardConfig.content.src },
357+
]);
358+
}
350359
}
351-
}
352360

353-
// TODO: this is really inefficient
354-
setDashboardJson((oldJSON) => ({
355-
...oldJSON,
356-
cards: oldJSON.cards.map((card) =>
357-
card.id === cardConfig.id
358-
? onCardChange
359-
? onCardChange(cardConfig, oldJSON)
360-
: cardConfig
361-
: card
362-
),
363-
}));
364-
};
361+
// TODO: this is really inefficient
362+
setDashboardJson((oldJSON) => ({
363+
...oldJSON,
364+
cards: oldJSON.cards.map((card) =>
365+
card.id === cardConfig.id
366+
? onCardChange
367+
? onCardChange(cardConfig, oldJSON)
368+
: cardConfig
369+
: card
370+
),
371+
}));
372+
},
373+
[availableImages, imagesToUpload, onCardChange]
374+
);
365375

366376
// Show the image gallery
367377
const handleShowImageGallery = () => setIsImageGalleryModalOpen(true);
@@ -379,34 +389,79 @@ const DashboardEditor = ({
379389
setIsImageGalleryModalOpen(false);
380390
};
381391

382-
const commonCardProps = (cardConfig, isSelected) => ({
383-
key: cardConfig.id,
384-
tooltip: cardConfig.description,
385-
availableActions: { clone: true, delete: true },
386-
onCardAction: (id, actionId, payload) => {
387-
if (actionId === CARD_ACTIONS.CLONE_CARD) {
388-
onDuplicateCard(id);
389-
} else if (actionId === CARD_ACTIONS.DELETE_CARD) {
390-
onRemoveCard(id);
391-
} else if (actionId === CARD_ACTIONS.ON_CARD_CHANGE) {
392-
handleOnCardChange(update(cardConfig, payload));
393-
}
394-
},
395-
tabIndex: 0,
396-
onKeyDown: (e) => handleKeyDown(e, onSelectCard, cardConfig.id),
397-
onClick: () => handleOnClick(onSelectCard, cardConfig.id),
398-
className: `${baseClassName}--preview__card`,
399-
isSelected,
400-
// Add the show gallery to image card
401-
onBrowseClick:
402-
cardConfig.type === CARD_TYPES.IMAGE && isNil(cardConfig.content?.src)
403-
? handleShowImageGallery
404-
: undefined,
405-
validateUploadedImage:
406-
cardConfig.type === CARD_TYPES.IMAGE
407-
? onValidateUploadedImage
408-
: undefined,
409-
});
392+
const commonCardProps = useCallback(
393+
(cardConfig, isSelected) => ({
394+
key: cardConfig.id,
395+
tooltip: cardConfig.description,
396+
availableActions: { clone: true, delete: true },
397+
onCardAction: (id, actionId, payload) => {
398+
if (actionId === CARD_ACTIONS.CLONE_CARD) {
399+
duplicateCard(id);
400+
} else if (actionId === CARD_ACTIONS.DELETE_CARD) {
401+
removeCard(id);
402+
} else if (actionId === CARD_ACTIONS.ON_CARD_CHANGE) {
403+
handleOnCardChange(update(cardConfig, payload));
404+
}
405+
},
406+
tabIndex: 0,
407+
onKeyDown: (e) => handleKeyDown(e, setSelectedCardId, cardConfig.id),
408+
onClick: () => handleOnClick(setSelectedCardId, cardConfig.id),
409+
className: `${baseClassName}--preview__card`,
410+
isSelected,
411+
// Add the show gallery to image card
412+
onBrowseClick:
413+
cardConfig.type === CARD_TYPES.IMAGE && isNil(cardConfig.content?.src)
414+
? handleShowImageGallery
415+
: undefined,
416+
validateUploadedImage:
417+
cardConfig.type === CARD_TYPES.IMAGE
418+
? onValidateUploadedImage
419+
: undefined,
420+
}),
421+
[duplicateCard, handleOnCardChange, onValidateUploadedImage, removeCard]
422+
);
423+
424+
const cards = useMemo(
425+
() =>
426+
dashboardJson?.cards?.map((cardConfig) => {
427+
const isSelected = cardConfig.id === selectedCardId;
428+
const cardProps = commonCardProps(cardConfig, isSelected);
429+
const dataItemsForCard = getValidDataItems
430+
? getValidDataItems(cardConfig)
431+
: dataItems;
432+
// if renderCardPreview function not defined, or it returns null, render default preview
433+
return (
434+
renderCardPreview(
435+
cardConfig,
436+
cardProps,
437+
setSelectedCardId,
438+
duplicateCard,
439+
removeCard,
440+
isSelected,
441+
handleShowImageGallery
442+
) ?? (
443+
<DashboardEditorCardRenderer
444+
key={cardConfig.id}
445+
{...cardConfig}
446+
{...cardProps}
447+
dataItems={dataItemsForCard}
448+
availableDimensions={availableDimensions}
449+
/>
450+
)
451+
);
452+
}),
453+
[
454+
availableDimensions,
455+
commonCardProps,
456+
dashboardJson,
457+
dataItems,
458+
duplicateCard,
459+
getValidDataItems,
460+
removeCard,
461+
renderCardPreview,
462+
selectedCardId,
463+
]
464+
);
410465

411466
return isLoading ? (
412467
<div className={baseClassName}>
@@ -518,31 +573,7 @@ const DashboardEditor = ({
518573
});
519574
}}
520575
supportedLayouts={['xl', 'lg', 'md']}>
521-
{dashboardJson.cards.map((cardConfig) => {
522-
const isSelected = cardConfig.id === selectedCardId;
523-
const cardProps = commonCardProps(cardConfig, isSelected);
524-
const dataItemsForCard = getValidDataItems
525-
? getValidDataItems(cardConfig)
526-
: dataItems;
527-
// if renderCardPreview function not defined, or it returns null, render default preview
528-
return (
529-
renderCardPreview(
530-
cardConfig,
531-
cardProps,
532-
onSelectCard,
533-
onDuplicateCard,
534-
onRemoveCard,
535-
isSelected,
536-
handleShowImageGallery
537-
) ??
538-
getCardPreview(
539-
cardConfig,
540-
cardProps,
541-
dataItemsForCard,
542-
availableDimensions
543-
)
544-
);
545-
})}
576+
{cards}
546577
</DashboardGrid>
547578
</ErrorBoundary>
548579
</div>

0 commit comments

Comments
 (0)