Cleaned up CreateItemModal implementation

[Vultraz]

CreateItemModal is no longer passed through to field views via prop. It can be imported from the admin UI package.

[changeset-bot]

🦋 Changeset is good to go

Latest commit: 71267f3

We got this.

This PR includes changesets to release 2 packages

Name	Type
@keystonejs/app-admin-ui	Minor
@keystonejs/fields	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[gautamsi]

can you also factor that out into separate package? like admin-ui-components @MadeByMike and I debated multiple times about the name of package but there is no conclusion.

[Vultraz]

I don't think that's a good idea. These are Admin UI components. We can't keep splitting everything into smaller and smaller packages, and each new package has a cost in terms of maintainability. 🤔

[gautamsi]

I had it first passed as prop to eliminate two different implementation for CreateItemModal. One in admin-ui and another in Relationship field.

Having each field which needs CreateItemModal and other component, dependent on app-admin-ui package is also not a good thing IMO

[Vultraz]

Two different implementations? I don't understand. There's no custom implementation in the Relationship field.

I agree it's not great to have the fields package depend on the admin ui, but the Relationship field already imports some stuff from the admin ui package (useList, for example), and using a prop model isn't great design. If more components were needed, for example, should each of them get a prop too? That feels even worse.

I think the problem here (and it's definitely beyond the scope of this issue), is the fact that we're trying to keep everything walled off when there is need for cross-dependence. The field views are meant for the admin ui, so it makes sense for them to be able to import admin UI things. But in that case, separation of concerns dictates that either they be in the same package (not possible under the current design), they depend on one another (seems people don't want this), or we create a third package... which just makes things harder to maintain and leads to more dependency version issues like have already been seen. :/

[gautamsi]

Two different implementations? I don't understand. There's no custom implementation in the Relationship field.

there was, see #1880

[Vultraz]

Oh, I hadn't seen that one.

[MadeByMike]

@Vultraz I like this. I know there was a bit of prop-drilling with passing the createItem mutation to the modal, but the create action was deliberately removed from the modal component to allow the possibility of generating a create item modal with a custom form handler.

Maybe this is ok and what we actually want to split out is just the form rendering bit inside Drawer as well as a more generic modal? What are your thoughts?

[Vultraz]

Agree on splitting out a more generic modal.

[Vultraz]

Conflict be gone.

[MadeByMike]

Approving this... but would like another pass in future at a generic modal and form generation