Add per Content Type enablement of same-page-preview

Fixed how the thirdPartySetting is used.

not sure if it is due to the patch or a problem with the way i apply the patch via composer-patches but when i try to apply the diff for 14 to an install o 10.1.x-dev i am getting "Could not apply patch! Skipping..."

That's why i've started relying upon the git repo / add new branch for MR approach. Would it be more helpful to you if I did the old patch creation workflow?

no staying with the MR workflow is totally fine. it usually works in 99,9% of the cases for me. the few times it failed it was due to an actual problem in the patch and i had it only once that the patch was fine but applying the patch still failed but no idea why. how do you apply patches? some other way than composer-patches?

oh yuck. i definitely need a good night rest tonight. just realized what the patch couldn't be applied, i've added it to drupal/core instead of drupal/same_page_preview. applies properly. will take a look now.

I usually follow the advice here → when manually applying patches. Or on projects with other devs, I'll include the patch via composer.

But with GitLab / MRs, I don't have to deal with any of that. I just use the instructions at the top of the issue on how to add the branch the MR creates.

found one error. i went into the article content type, left the enable same page preview unchecked and saved the content types edit form and then went to the article node add form and got this:

The website encountered an unexpected error. Please try again later.

Error: Call to a member function getType() on null in samePagePreviewActive() (line 173 of modules/contrib/same_page_preview/same_page_preview.module).
same_page_preview_form_node_form_alter(Array, Object, 'node_article_form') (Line: 545)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_article_form') (Line: 838)
Drupal\Core\Form\FormBuilder->prepareForm('node_article_form', Array, Object) (Line: 282)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 698)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

did the same steps for the basic page content type only difference i'Ve checked the enable same page preview this time. but in the end same result aka same error (only different form) when i tried to access the basic page node add form. (the project in ddev is using php 8.2)

The website encountered an unexpected error. Please try again later.

Error: Call to a member function getType() on null in samePagePreviewActive() (line 173 of modules/contrib/same_page_preview/same_page_preview.module).
same_page_preview_form_node_form_alter(Array, Object, 'node_page_form') (Line: 545)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_page_form') (Line: 838)
Drupal\Core\Form\FormBuilder->prepareForm('node_page_form', Array, Object) (Line: 282)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 698)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I usually follow the advice here when manually applying patches. Or on projects with other devs, I'll include the patch via composer.

But with GitLab / MRs, I don't have to deal with any of that. I just use the instructions at the top of the issue on how to add the branch the MR creates.

yep the composer based patch workflow i usually use. (as noted in #8 📌 Add per Content Type enablement of same-page-preview Fixed i just tried to apply the diff to core instead of same_page_preview - an embarrassing oversight). and the first link i haven't known yet. i think i will give the git apply workflow also try as a failsafe just in case. thanks!

Rebased this branch (but might have to do that again). Not sure if it incorporates @rkoller's feedback. I'll take another look

Hi @cosmicdreams , Applied your MR. It was successfully added Enable Same Page Preview checkbox on content types. Need RTB+1

the wsod described in #10 📌 Add per Content Type enablement of same-page-preview Fixed still happens if you are trying to create a new node for a content type that has the enable same page preview checkbox unchecked (i have to correct myself the error happens in general also for content types that have the enable checkbox checked)

Think I found a flaw in how the module sets expectations. There might be an issue with testing a content type after the patch is applied (without going to the content type and saving the config. I'll retest, but I think my fix might cover it.

As already written on Slack. Changing the Drupal version to 10.0.x fixed the errors in #10 and #15. The only slight nitpick i would have now that I wasn't distracted by any error is the description of the checkbox on the node edit form of the content type. Currently you have:

enable same page preview

for the checkbox label and:

enable same page preview for this content type

for the description. that is redundant. you have enable same page preview twice and the description just appends the for this content type snippet at the end. me as a user wouldnt necessarily know what to expect. currently without knowing the functionality i would expect to directly see a preview window in what ever form on the node edit page. but instead only a toggle preview button is added. therefore changing the description to something like:

Adds a Toggle Preview button to the node edit form for this content type.

might do the trick?

@rkoller for the description how about

'Allow advanced preview feature for this content type.'

Not perfect yet. But better than my suggestion and way better than the current description. And since the functionality and settings get extended and handled differently from phase 2 on i think your suggestion would be ok for now. and the micro copy could be revisited during phase 2 at a later point so things can advance. sounds good?

edit: ah you already pushed the description change and changed the status while i was writing my comment. then i set the status to rtbc.

Yep all of our strings are in flux.

Looks like I will have to create a new issue to get this in 2.0.x

Automatically closed - issue fixed for 2 weeks with no activity.