-
Notifications
You must be signed in to change notification settings - Fork 25
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
fix(studio-ui-codegen-react): collection items props takes precedent #96
Conversation
...ges/studio-ui-codegen-react/lib/__tests__/__snapshots__/studio-ui-codegen-react.test.ts.snap
Show resolved
Hide resolved
export default function CollectionOfCustomButtons( | ||
props: CollectionOfCustomButtonsProps | ||
): JSX.Element { | ||
const { width, backgroundColor, buttonColor } = props; |
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.
I think we had intended to always allow the items of a collection to be overridden and just not call datastore if so. Might be good to ask at standup.
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.
Are you saying if items
is passed in props
then props.items
should be used in place of const { boundItems } = useDataStoreBinding...
?
So even if we have boundItems
defined props.items
should take precedent?
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.
I believe that is the intended functionality. @renebrandel can you confirm?
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.
I think we'll have to bring it up as "parking lot item" during standup. @dpilch
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.
I just talked with Rene to nail this down. Any component that is a collection will always expose a prop where items can be passed in to be rendered by the collection. If a predicate is provided and that prop is empty, a datastore call should be made and that data used to render.
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.
I'm looking into the fix for this.
I think it would look something like this.
export type CollectionOfCustomButtonsProps = {
items?: any[],
}
export default function CollectionOfCustomButtons(
props: CollectionOfCustomButtonsProps
): JSX.Element {
const { items } = props;
const buttonUserFilter = {
and: [
{ field: "age", operand: "10", operator: "gt" },
{ field: "lastName", operand: "L", operator: "beginsWith" },
],
};
const displayedItems = items !== undefined
? items
: useDataStoreBinding({
type: "collection",
model: User,
criteria: buttonUserFilter,
}).buttonUser;
return (
<Collection
items={displayedItems}
...
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.
LGTM!
f997fdc
to
b21bb60
Compare
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.
LGTM
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.
LGTM
Resolves #90
items
prop take precedent overuseDataStoreBinding
See snapshot tests for examples