getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.31k stars 168 forks source link

Title field for file blueprint value disappears in panel #4140

Closed mrvicis closed 2 years ago

mrvicis commented 2 years ago

Starterkit 3.6.2

To reproduce

  1. Create default.yml blueprint for files
  2. Add a title text field in blueprint
  3. Upload random file go to edit it's meta
  4. Write some title and save changes.
  5. Press F5/Cmd+R — title disappears.

[!] But Title value saves in .txt file.

Feb-08-2022 15-31-14

afbora commented 2 years ago

I guess title reserved prop for all models blueprint:

Parent blueprint of models: https://getkirby.com/docs/reference/objects/cms/blueprint/title

Child blueprints: https://getkirby.com/docs/reference/objects/cms/page-blueprint/title https://getkirby.com/docs/reference/objects/cms/file-blueprint/title https://getkirby.com/docs/reference/objects/cms/user-blueprint/title

mrvicis commented 2 years ago

@afbora ok, thank you, I'll try to name files in another way.

Also I followed this recipe: https://getkirby.com/docs/cookbook/templating/virtual-pages-image-gallery#file-blueprint Title field seemed to work here.

afbora commented 2 years ago

Thanks for point out the recipe. I'll check for a regression by trying this recipe.

afbora commented 2 years ago

I've checked on 3.5.8, works great and doesn't work on 3.6.0. I guess this is a regression introduced with 3.6. Because I couldn't find any breaking change note about that.

afbora commented 2 years ago

Summary

I finally found the cause of the problem. When content/create is dispatched, the title data is deleted. This deletion is designed for page and site model, but file models are included in this dispatch process in 3.6. But I have no idea how we can exclude the file models.

You will see better when you follow the links below in order.

https://github.com/getkirby/kirby/blob/3.6.2/panel/src/components/Views/ModelView.vue#L65-L69 https://github.com/getkirby/kirby/blob/3.6.2/panel/src/store/modules/content.js#L253-L255

afbora commented 2 years ago

Possible Solutions

Exclude path that contains /files

if (
    (model.id.startsWith("/pages/") || model.id.startsWith("/site")) &&
    model.id.contains("/files") === false
) {
  delete model.content.title;
}

Add new type data for models to dispatch payload

if (
    (model.id.startsWith("/pages/") || model.id.startsWith("/site")) &&
    model.type !== 'file'
) {
  delete model.content.title;
}

Which is more stable? Do you have a better solution suggestion?

afbora commented 2 years ago

What do you think about above solution and adding this bug-fix into 3.6.3 because of it's regression?

bastianallgeier commented 2 years ago

I fixed it slightly differently by unsetting the title fields already on the backend. It's a lot easier there and we no longer need to base it on the URL path, which was always super hacky.

bastianallgeier commented 2 years ago

Damn, I just realized that this would be a breaking change. Too bad. I will close the PR and try a different approach. I think your type suggestion would be a great alternative, but unfortunately we already use model.type for file types

bastianallgeier commented 2 years ago

Ok, I think I have a solution that fixes it without any breaking changes and is still pretty clean.

bastianallgeier commented 2 years ago

✅ will be fixed in 3.6.4