Closed (fixed)
Project:
Lupus Decoupled Drupal
Version:
1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
13 Nov 2024 at 15:23 UTC
Updated:
30 Dec 2024 at 14:24 UTC
Jump to comment: Most recent
Comments
Comment #3
useernamee commentedComment #4
fagothanks! please see my comments!
Comment #5
useernamee commentedI've implemented most of the PR comments.
I think the one regarding the documentation was already covered by ld_form README file.
I added additional
typeattribute to custom element because that data was lost after I utilized more ld_form code and simplified the output.I was surprised by isApiResponse method code quadruplication. This should probably be caught before.
New output looks like:
Comment #6
arthur_lorenz commentedBy default webform uses the confirmation type "page". However confirmation pages are not supported, resulting in a redirect to Drupal's frontend. I see multiple possible solutions:
Comment #7
fagoIf we could easily support confirmation pages, that would be best. Else I guess we should at least have a working default.
Generally, we are not warning users of not supported stuff, that would be lots of work, would it? Or should we make an exception for that crucial setting and grey-out/disable not supported options? This means, enabling the module takes over all webforms and considers them decoupled. Maybe it would be nice to allow configuring that, but again, complexity, so I think it's ok to keep it simple and make all webforms deocupled once turned on.
Comment #8
useernamee commentedComment #9
useernamee commentedEnabling confirmation was not problematic but there's a strange access check in webform that allowed me to access confirmation page only when logged in as admin:
/webform/src/WebformEntityAccessControlHandler::checkAccess
Comment #10
useernamee commentedOk, I figured it out and added a section to the
README.mdfile.Comment #11
useernamee commentedComment #12
arthur_lorenz commentedThx, comments were adressed. I tested by creating a webform including 2 pages and confirmation page -> works like documented.
Comment #14
useernamee commentedMerged.
Bug hunting season starts now.