tinacms / tinacms

A fully open-source headless CMS that supports Markdown and Visual Editing
https://tina.io
Apache License 2.0
11.32k stars 573 forks source link

feat: datalayer folders #3750

Closed kldavis4 closed 1 year ago

kldavis4 commented 1 year ago

what

Implements folder indexing with collections and updates the admin user interface to enable a folder mode.

Screenshot 2023-04-13 at 4 18 18 PM
changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: eeedcfd303a8c629dbaecdd1b1d4d48c8f037e3b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages | Name | Type | | ---------------------------- | ----- | | @tinacms/starter-iframe | Patch | | @tinacms/schema-tools | Patch | | @tinacms/graphql | Patch | | @tinacms/toolkit | Minor | | e2e-next | Patch | | @tinacms/cli | Patch | | tinacms | Minor | | @tinacms/mdx | Patch | | @tinacms/datalayer | Patch | | starter-basic-iframe | Patch | | @tinacms/app | Patch | | starter-empty | Patch | | @tinacms/self-hosted-starter | Patch | | kitchen-sink-starter | Patch |

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

spbyrne commented 1 year ago

Here's where I'm at with the UI. It sounds like people like to remove the toggle, though. It'll use a folder view unless the user is using search (if I remember correctly from the discussion).

Screenshot 2023-04-04 at 3 28 25 PM
jeffsee55 commented 1 year ago

but do you think we should we also add the folder to Generated queries

I think we're treating this as more of an internal API for now so that's not necessary. It's also a little bit strange because the results of the query are all of type Document, when it reality it's a directory. Might be something we want to clean up beforehand.

kldavis4 commented 1 year ago

I think the contextual editing links are not working for documents in sub-folders in the kitchen-sink example. If I click on a root level post it opens fine. If I click on a document in a folder, it routes to ~/:collection/:filename and throws an error: Unable to find record content/:collection/:filename.mdx. If I manually update the url to include the folder name (~/:collection/:folder/:filename) I get an empty form and a 404 error.

Is this something that the route mapper is responsible for handling (if configured)?

@logan-anderson @jeffsee55 @jamespohalloran

kldavis4 commented 1 year ago

but do you think we should we also add the folder to Generated queries

I think we're treating this as more of an internal API for now so that's not necessary. It's also a little bit strange because the results of the query are all of type Document, when it reality it's a directory. Might be something we want to clean up beforehand.

I'm not sure I'm following this 🤔

logan-anderson commented 1 year ago

Re: @kldavis4

I'm not sure I'm following this 🤔

I am talking about potentially adding the file filter to get<CollectionName>Connections.

EX

getPostConnnections(folder: "nested") {
   ...
}

I agree with @jeffsee55 It can stay as an internal API for now but it can remain on the radar.

Is this something that the route mapper is responsible for handling (if configured)?

@kldavis4 I think this is an issue with the implemtion in getStaticProps. I can take a look.

kldavis4 commented 1 year ago

Re: @kldavis4

I'm not sure I'm following this 🤔

I am talking about potentially adding the file filter to get<CollectionName>Connections.

EX

getPostConnnections(folder: "nested") {
   ...
}

I agree with @jeffsee55 It can stay as an internal API for now but it can remain on the radar.

^ Not this. This:

It's also a little bit strange because the results of the query are all of type Document, when it reality it's a directory. Might be something we want to clean up beforehand.

logan-anderson commented 1 year ago

Re: @kldavis4

I'm not sure I'm following this 🤔

I am talking about potentially adding the file filter to get<CollectionName>Connections. EX

getPostConnnections(folder: "nested") {
   ...
}

I agree with @jeffsee55 It can stay as an internal API for now but it can remain on the radar.

^ Not this. This:

It's also a little bit strange because the results of the query are all of type Document, when it reality it's a directory. Might be something we want to clean up beforehand.

@kldavis4 I am not sure either.

logan-anderson commented 1 year ago

@kldavis4 I made pr into this branch that updates the example to work with folders.

kldavis4 commented 1 year ago

thanks @logan-anderson, can you update kitchen-sink as well?

jeffsee55 commented 1 year ago

I'm not sure I'm following this 🤔

@kldavis4 what I mean is that if we add the folder param won't that change the results of user's queries if one of the results was a folder?

kldavis4 commented 1 year ago

I'm not sure I'm following this 🤔

@kldavis4 what I mean is that if we add the folder param won't that change the results of user's queries if one of the results was a folder?

right, ok that makes sense. Do you have an idea on how to resolve that?

jeffsee55 commented 1 year ago

Do you have an idea on how to resolve that?

When I tried to add the proper GraphQL type to support this it would have broken the current way of querying so I don't think there's much we can do. Maybe we should have a separate field on the collection for the feature specifically 🤔

kldavis4 commented 1 year ago

@kldavis4 I made pr into this branch that updates the example to work with folders.

@logan-anderson can you make this change in the kitchen-sink starter?

logan-anderson commented 1 year ago

@kldavis4 I made a PR: https://github.com/tinacms/tinacms/pull/3840 to add that

logan-anderson commented 1 year ago

@kldavis4 Found a small issue: https://www.loom.com/share/a2e114f1e6e343aa9fc7c9a7eec2e9d0

kldavis4 commented 1 year ago

@kldavis4 Found a small issue: https://www.loom.com/share/a2e114f1e6e343aa9fc7c9a7eec2e9d0

@logan-anderson seems to be a bit of a weird race condition that I'm not sure the best way to resolve. I inserted a 10ms delay and it seems to have fixed it, but not sure if there is a better solution