-
Notifications
You must be signed in to change notification settings - Fork 9
1042 relation data path formatter #1447
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.
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
base: 1.x
Are you sure you want to change the base?
Conversation
return acc | ||
}, {}) | ||
|
||
const { data } = await store.dispatch(api.endpoints.dataObjectFormatPath.initiate({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some kind of caching here so that we don't request items with the same context information again after we already requested it? (so just remember the result in a local ref or so)
assets/js/src/core/modules/data-object/hooks/use-data-object-helper.ts
Outdated
Show resolved
Hide resolved
...c-types/definitions/objects/data-related/components/many-to-many-relation/hooks/use-value.ts
Outdated
Show resolved
Hide resolved
<Tag | ||
bordered={ false } | ||
color="geekblue" | ||
><SanitizeHtml html={ info.getValue() ?? '' } /></Tag> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...ent/dynamic-types/definitions/objects/data-related/components/many-to-many-relation/grid.tsx
Outdated
Show resolved
Hide resolved
@@ -79,7 +81,7 @@ export const ManyToManyRelation = (props: ManyToManyRelationProps): React.JSX.El | |||
} | |||
}, [JSON.stringify(props.value)]) | |||
|
|||
if (props.isLoading === true) { | |||
if (isLoading || props.isLoading === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loading state here seemed to be a good idea, but it's a bit annoying that the full data type has a loading state - especially when a new item is added via the tree. The grid data type itself also provides a loading prop. Maybe it's better to just set the grid itself to loading instead of everything? Not sure how easy it's possible but in a perfect world we would see the loading spinner directly on row level for newly added items (and only for the initial load we use the global one).
@@ -33,22 +38,47 @@ interface UseValueReturn { | |||
addItems: (items: ManyToManyRelationValueItem[]) => void | |||
addAssets: (assets: Asset[]) => Promise<void> | |||
maxRemainingItems?: number | |||
pathFormatterClass?: string | null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we have the loading spinner + the triggered format-path
request also when no path formatter is configured. We need to find a way to skip the path formatter logic when no pathFormatterClass
is defined.
body: { | ||
objectId: dataObjectId, | ||
targets, | ||
context: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be a bit tricky but we need to somehow find a way to respect the different possible context cases:
|
Changes in this pull request
Resolves #1042
Additional info