getgrav / grav-plugin-admin

Grav Admin Plugin
http://getgrav.org
MIT License
355 stars 227 forks source link

Grav 1.7.3 + Admin 1.10.3, can't change Page template twice and re-save #2037

Closed paulhibbitts closed 3 years ago

paulhibbitts commented 3 years ago

This is based of a report in the Discourse forum: https://discourse.getgrav.org/t/cant-save-modification/14835/2

I was able to reproduce it locally with latest 1.7.3 build on Mac Chrome and Firefox.

Steps: 1) Edit page 2) Change page template 3) Save 4) Change page template again 5) Save (this does not seem to work)

Please let me know if any more info is helpful etc. Paul

Karmalakas commented 3 years ago

Added a comment on same topic. I also couldn't save a template change no matter what, until I filled Title field in Content tab. Then it saved.

Could it be it won't detect a changed template field and since nothing changes in Content it just says form is saved, but it really isn't?

Karmalakas commented 3 years ago

Might be related - https://github.com/getgrav/grav-plugin-admin/issues/2056

rhukster commented 3 years ago

I can't replicate this one with the steps provided @paulhibbitts

rhukster commented 3 years ago

I'm on Grav 1.7.5 / Admin 1.10.3

rhukster commented 3 years ago

BTW this is not related to #2056 that is most certainly an issue for List field only.

paulhibbitts commented 3 years ago

Hi @rhukster , here is video to help demo the issue:

https://user-images.githubusercontent.com/1812771/106789679-70e82b80-6607-11eb-94fd-b52c983352fc.mov

I used my most recent build of my skeleton as the test site (updated to most recent Grav/Admin): https://github.com/hibbitts-design/grav-skeleton-course-hub/releases/download/v3.2.9/grav-skeleton-open-matter-course-hub-site.zip

Any luck replicating with the above?

Thanks very much, Paul

rhukster commented 3 years ago

I tried even with the leaving and going back to the page like you did, works fine for me. Please check your "Console" in web dev tools to see if you are getting a JS error.

paulhibbitts commented 3 years ago

Hmm... I just tried locally and saw the same issue but nothing in the Console?

Capto_Capture 2021-02-03_10-27-52_AM

I will next try online site just in case.

Thanks, Paul

paulhibbitts commented 3 years ago

Ok, I've done some more testing. For some reason both my Open Course Hub and Open Publishing Space skeletons (using two different themes) show this issue on my end. However, both the Grav Admin and Ole's Project Space do not.

@Karmalakas what skeleton/theme are you having this issue with? Any ideas of what might be going on @rhukster ?

Happy to do more testing etc. Paul

paulhibbitts commented 3 years ago

UPDATE: If I change my active theme to Bootstrap4 (from my inherited theme) Save works! So, perhaps this is some sort of a theme/blueprint issue that might be related to 1.7?

paulhibbitts commented 3 years ago

Re: perhaps issue is related to theme blueprints, is there anything off with this one re: 1.7 @rhukster ?

title: Open Matter Item Options
'@extends':
    type: default
    context: blueprints://pages

form:
  fields:

    tabs:
      type: tabs
      active: 1

    <truncated>

Full blueprint at https://github.com/hibbitts-design/grav-theme-bootstrap4-open-matter/blob/master/blueprints/item.yaml

Karmalakas commented 3 years ago

I'm using my own very simple theme with only one event and 3 custom template blueprints with standard fields for content editing (3-6 fields each)

    public static function getSubscribedEvents()
    {
        return [
            'onTwigInitialized'     => ['onTwigInitialized', 0],
        ];
    }

    public function onTwigInitialized()
    {
        $twig = $this->grav['twig'];

        $form_class_variables = [
            'form_outer_classes' => 'form-horizontal',
            'form_button_outer_classes' => 'button-wrapper',
            'form_field_wrapper_classes' => 'form-floating',
            'form_field_inline_error_classes' => 'invalid-feedback',
        ];

        $twig->twig_vars = array_merge($twig->twig_vars, $form_class_variables);
    }
paulhibbitts commented 3 years ago

My themes also have some custom PHP going on: https://github.com/hibbitts-design/grav-theme-bootstrap4-open-matter/blob/master/bootstrap4-open-matter.php

I've been following all the 1.7RC stuff so I thought no further changes were needed (if this might be related).

What does your page blueprint look like @Karmalakas where you have this issue?

Karmalakas commented 3 years ago

What does your page blueprint look like @Karmalakas where you have this issue?

Oh if only I could remember now :D As soon as I filled a title and it saved, I forgot about it :D But I think it was this template I couldn't save:

title: Contact form
'@extends': default

form:
  fields:
    tabs:
      fields:
        content:
          fields:

            header.media_order:
              unset@: true

            header.form_content:
              type: markdown
              label: "Form text"
              validate:
                type: textarea
paulhibbitts commented 3 years ago

Hey @rhukster @Karmalakas , maybe I have a lead!

I notice that when I've switched to another template, and then that template has required items, a second save (with another template is not working)... for example, here is External trying to change to Item:

2021-02-03_11-44-50

And save is not working second time. BUT going to Content I see this:

2021-02-03_11-44-59

To summarize: 1) template is Item 2) change template to External, and save 3) change templete back to Item, try to save but save does not work 4) previous use of External needing required field (look on Content tab) and I think thus preventing second change/Save

Which I think was preventing the change to template (second time) being saved... any thoughts? Can you confirm @Karmalakas ?

mahagr commented 3 years ago

Required fields do prevent the save button from firing, so it is what happens here. When you change the type, Grav only renames the file and reloads the page. When you try to save a second time, your page does not pass the tests and you cannot click the save button.

This is what is expected and happened with older versions, too.

I think that there should be some warning if required fields are failing in other tabs, I find it annoying that the button just stops working without any indication of what is going on.

Also in the future, changing the page template should probably reload the page with the new form fields pre-filled with the current data from your browser.

paulhibbitts commented 3 years ago

Thanks very much for looking into it and verifying the behaviour @mahagr . Your idea of some sort of indication of why the save fails sounds helpful too.