wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.34k stars 191 forks source link

[WIP] Improve form repeaters #448

Open jaxwilko opened 2 years ago

jaxwilko commented 2 years ago

This is a list of notes for things I would like to add, if anybody has any suggestions please add a comment :)

AIC-BV commented 2 years ago

I noticed that the lazy loading does not work in fields.yaml for a repeater field. It returns an array but the lazy loading expects a string. Could be an improvement! 😄

EDIT: Hmm forget what I said, its more a bug, it works but in some cases it doesn't. Can't find the reason I'll have to dig deeper.

AIC-BV commented 2 years ago

I noticed that the lazy loading does not work in fields.yaml for a repeater field. It returns an array but the lazy loading expects a string. Could be an improvement! 😄

EDIT: Hmm forget what I said, its more a bug, it works but in some cases it doesn't. Can't find the reason I'll have to dig deeper.

So I did some deeper digging and found out the following:

Error: htmlspecialchars(): Argument #1 ($string) must be of type string, array given Triggered by: I think it happens when you have a repeater like this in the database.

[{
  "name": "My name",
  "remarks": "My remarks",
  "image": "data:image\/jpeg;base64, ...",
  "production": {
      "drawing": "file.pdf",
      "approved_status": 1,
      "remarks": "Ok."
  }
}]

I can understand it errors on a base64. The base64 was only temporary and will be removed and changed to an image path (no worries! 😅) But I'm pretty sure it happens on the json object.

I can read the JSON object like this: Note that I am using this inside a repeater

production[drawing]:
  type: mediafinder

production[approved_status]:
  type: dropdown

production[remarks]:
  type: textarea

This is definitely not a required improvement, could work around by splitting the JSON object or not using lazy 😄 Just wanted to let you guys know the issue!

github-actions[bot] commented 2 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

AIC-BV commented 2 years ago

Finally found this again! 😌 @jaxwilko I ran into the issue described above again, and it took me a while to figure it out. When I found the solution, I realised I had this problem in the past too...

I am using repeater groups, in which I created a pretty block using a Nestedform because I wanted tabs to make the repeater more compact: image

In fields.yaml

blocks:
            tab: aic.blog::lang.form.blocks
            titleFrom: title
            prompt: aic.blog::lang.form.blocks_add
            type: repeater
            cssClass: secondary-tab
            stretch: true
            groups: ~/plugins/aic/extendwinterpages/meta/blocks.yaml

In blocks.yaml

block_image_with_text:
    name: Image with text
    icon: icon-address-card-o
    fields:
        section:
            label: Image with text
            type: section
        type:
            label: Image
            type: balloon-selector
            default: left
            options:
                left: Left
                right: Right
        data:
            type: nestedform
            usePanelStyles: false
            cssClass: custom-nested-form
            form:
                tabs:
                    icons:
                        Text: icon-keyboard
                        Image: icon-image
                        Link: icon-link
                    fields:
                        title:
                            tab: Text
                            label: Title
                            default: Title
                            type: text
                        text:
                            tab: Text
                            label: Text
                            default: Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris nec bibendum nisl. Quisque iaculis feugiat iaculis. Vestibulum lacinia massa vitae nulla blandit, at vestibulum dui malesuada.
                            type: richeditor
                            size: small
...
...

This outputs the same type of data as in my post on 24 Feb. It was giving an error when using lazy for the blocks tab Error: htmlspecialchars(): Argument #1 ($string) must be of type string, array given The workaround is NOT using lazy for the blocks tab!

AIC-BV commented 2 years ago

What I am also experiencing with the code above, is that when I fill in blog.blocks.block_image_with_text.data.form.tabs.fields.title, it also sets my blog.title (in fields.yaml) to the same value as blog.blocks.block_image_with_text.data.form.tabs.fields.title

The workaround is NOT using title in my blocks, but, for example, block_title instead

LukeTowers commented 2 years ago

@AIC-BV submit that as a separate issue please, ideally with a PR to the wintercms/wn-test-plugin demonstrating the issue.

AIC-BV commented 1 year ago

@AIC-BV submit that as a separate issue please, ideally with a PR to the wintercms/wn-test-plugin demonstrating the issue.

https://github.com/wintercms/wn-translate-plugin/issues/44 without the PR but a better explanation and some more research in it 👼

AIC-BV commented 1 year ago

This is a list of notes for things I would like to add, if anybody has any suggestions please add a comment :)

  • Add new item under each item (rather than at the bottom and dragging it up)
  • Speed up item generation
  • Allow items to be dragged between scopes if acceptable
  • More span options? (maybe)

Is there a version of this that I can test? It could potentionally solve my other issue, because reordering wouldn't be such an important feature if I can add a new item under each item (https://github.com/wintercms/winter/issues/715)

There's also this issue which could perhaps be tackled in the rework, although i'm not 100% sure its related to the repeater: https://github.com/wintercms/winter/issues/716

github-actions[bot] commented 1 year ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

lex0r commented 1 year ago

A minor UX improvement for which I have an implementation is showing an empty repeater form automatically if there are no existing repeater items yet. This helps avoid an extra click for cases when at least one repeater element is required before the parent form can be saved in principle.

PR: https://github.com/wintercms/winter/pull/920