Skip to content

LPD-24670 WIP. #5110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

LPD-24670 WIP. #5110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b587ce
LPD-24670 WIP. Check selected item has no visualization modes. In the…
dsanz Oct 7, 2025
5254bc6
LPD-24670 Avoid mutating item by hoisting setItemComponentPropsTransf…
dsanz Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion ...et-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@ const views = [
symbol: 'symbol',
title: 'label',
},
setItemComponentProps: ({item, props}: {item: any; props: any}) => {
if (
!item.dataSetToDataSetCardsSections.length &&
!item.dataSetToDataSetTableSections.length &&
!item.dataSetToDataSetListSections.length
) {

return {
...props, // we need to avoid item mutation
item: {
...item,
symbol: 'warning',
tooltip: 'perico'
}
}
Comment on lines +38 to +45
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first feeling about this was that it wasn't right, because API was intended to modify props of component implementing the item, <ClayList.Item>. It does make sense to shift target of customization to <ListItem>, our own wrapper of the component. It makes customization of our implementation more flexible. But. it's a tradeoff. We lose future-proof aspect of the solution, where devs would automatically be able to modify future props of <ClayList.Item>. It might make sense to add a prop specifically for that purpose. Something like

const props = {
		className,
		clayListItemProps, // new prop
		item,
		items,
		onItemSelectionChange,
		ref:dropRef,
		schema,
	};

<ListItem
		{...props}
		{...(activeView.setItemComponentProps?.({item, props}) ?? {})}

that ends up in

<ClayList.Item
	className={classNames(className, {
		active: selectedItemsValue?.includes(itemId),
	})}
	flex
	{...clayListItemProps}
>

In general, mutating item is not good practice, but I think it's acceptable this close to leaf usage.

What do you think?

Copy link
Collaborator Author

@dsanz dsanz Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this shift makes item not mutated indeed.

Note how first commit does item.foo = bar whereas second one just provides a different item. So no internal FDS state is mutated.

It's true that we are changing the item we pass to the child component. I believe there's a slight difference between this and mutating the item.

And yes, what you say makes sense to me, we need to open the possibility to drill down props in the "real" target component here. @chestofwonders pls add this to #5122 so that devs have access to the underlying <ClayList.Item> so that we get the best of both worlds

Thank you @markocikos !


}

return props;
},
},
];

Expand Down Expand Up @@ -67,7 +87,12 @@ const FDSAdminItemSelector = ({
<ClayModal.Body>
<FrontendDataSet
{...FDS_DEFAULT_PROPS}
apiURL={getDataSetResourceURL({})}
apiURL={getDataSetResourceURL({
params: {
nestedFields:
'dataSetToDataSetCardsSections, dataSetToDataSetTableSections, dataSetToDataSetListSections',
},
})}
id={`${namespace}FDSAdminItemSelector`}
onSelectedItemsChange={(
selectedItems: Array<ISelectedItem>
Expand Down
26 changes: 15 additions & 11 deletions ...-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ const ListItem = forwardRef<HTMLLIElement, any>(
selectionType,
} = useContext(FrontendDataSetContext);

const [viewsContext] = useContext(ViewsContext);

const activeView: IView = viewsContext.activeView;

const {description, image, sticker, symbol, title, titleRenderer} =
schema;

Expand All @@ -102,7 +98,6 @@ const ListItem = forwardRef<HTMLLIElement, any>(
return (
<ClayList.Item
{...props}
{...(activeView.setItemComponentProps?.({item, props}) ?? {})}
ref={ref}
>
{selectable && (
Expand Down Expand Up @@ -194,14 +189,23 @@ const ListItemOptionalDropTarget = ({
}) => {
const {className, dropRef} = useFDSDrop({item});

const [viewsContext] = useContext(ViewsContext);

const activeView: IView = viewsContext.activeView;

const props = {
className,
item,
items,
onItemSelectionChange,
ref:dropRef,
schema,
};

return (
<ListItem
className={className}
item={item}
items={items}
onItemSelectionChange={onItemSelectionChange}
ref={dropRef}
schema={schema}
{...props}
{...(activeView.setItemComponentProps?.({item, props}) ?? {})}
/>
);
};
Expand Down