Add support for forms

Created on 24 January 2023, over 1 year ago
Updated 15 June 2024, 1 day ago

Problem/Motivation

Process and generate Drupal forms with custom-elements viable output.

Proposed resolution

drunomics has a pre-existing solutions built internally, we should build upon that and add it as lupus_decoupled_form sub-module.

Question is whether we should add some conditional routes/code for adding support for things like contact module support or start adding individual modules for case like that.

πŸ“Œ Task
Status

Needs review

Version

1.0

Component

Code

Created by

πŸ‡¦πŸ‡ΉAustria fago Vienna

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    ok, pushed the existing code into the issue form. It uses "ldp_" namespace, so we need to move that to lupus_decoupled namespace.

    https://git.drupalcode.org/issue/lupus_decoupled-3336148/-/tree/05d0f925...
    https://git.drupalcode.org/issue/lupus_decoupled-3336148/-/tree/05d0f925...

    It has a good README which should explain how it works pretty well. The code supports a theme-switch for a clean html-markup of forms. We've been using it togoether with https://www.drupal.org/project/lupus_stark β†’ - so that makes sense to add here and auto-enable somehow if found.

    On the frontend-side I'm going to paste the developed components into an issue of the nuxtjs module, we probably should add it to the module, e.g. auto-register those components. --> Find them here: https://github.com/drunomics/nuxtjs-drupal-ce/issues/101

  • First commit to issue fork.
  • πŸ‡¬πŸ‡§United Kingdom m0d

    Hi! Coming from the issue https://www.drupal.org/project/lupus_ce_renderer/issues/3338361 ✨ Adding support to User login and User password forms Closed: duplicate . I've renamed the modules, so they follow the pattern lupus_decoupled. I'll try and adapt my code for user forms in a similar way to your contact module.

  • Status changed to Needs review over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom m0d
  • πŸ‡¬πŸ‡§United Kingdom m0d

    Added support to user login and user password reset forms https://git.drupalcode.org/issue/lupus_decoupled-3336148/-/commit/f6be7e...

  • Merge request !30Issue 3336148 - Form integration β†’ (Open) created by m0d
  • Status changed to Needs work over 1 year ago
  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    thank you for working on this!

    I've reviewed the MR and left a couple of comments.

  • Status changed to Needs review over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom m0d
  • Status changed to Needs work over 1 year ago
  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    thank you! I've done other review, please check my comments!

  • Status changed to Needs review about 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom m0d

    I addressed almost all your feedback. I added a couple of questions.

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    thank you for your changes! I think this is getting close!

    I've reviewed it again and left you a few comments.

    This piece of code redirects any POST responses to login, password reset or register forms in HTML format so they can be redirected to the frontend URL. We can't deal with them in \Drupal\lupus_decoupled_form\Controller\CustomElementsFormController::getContentResult since the request won't get there. It'd get there if the format was 'custom_elements'.

    I see, sure makes sense! I missed that. I think it's a bit confusing in general, when the form goes through a different flow/controller during build and submission. Could we simply set the form action with a /ce-api prefix so it goes via the custom-elements version of the controller again? Then it would be consistent + we should be able to remove the EventSubscriber in favour of issuing the redirect in the controller?

    Could you please provide those URLs? I've looked them up but I don't know which one's you're referring to.

    The components are at https://github.com/drunomics/nuxtjs-drupal-ce/issues/101 - until they get incorporated there properly. Let's simply link to this issues for ow.

  • Status changed to Needs work about 1 year ago
  • πŸ‡¦πŸ‡ΉAustria fago Vienna
  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    Given this a little more thought.

    I see, sure makes sense! I missed that. I think it's a bit confusing in general, when the form goes through a different flow/controller during build and submission. Could we simply set the form action with a /ce-api prefix so it goes via the custom-elements version of the controller again?

    Actually, having only one form flow is the argument *against* submitting to /ce-api prefix. Alterations do the form should happen in general and not only when custom-elements is active. That way the form will degrade properly without JS and the frontend is free to handle the form submission via ajax or traditional form submit.

    So, having the alteration done generally, in lupus_decoupled_user_form_form_alter() seems to be the right thing to do. +1 on keeping it like that!

    That said, the MR seems almost ready!

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    I've been looking at this with forms that work via both the frontend and backend and think there is a bug in the URL handling both here and in BackendApiRequest.

    Here, we explictly add a slash to the form action - but forms generated in the backend already start with a slash, so you end up with double slashes, e.g. https://example.com//user/login. This causes issues on POSTs because the double slash is redirected to a single-slash URL and the POST data is lost. If I fix this then it triggers a different bug:

    Forms generated in the frontend via the /ce-api route incorrectly remove the first slash. So /ce-api/user/login on becomes just user/login in the form action - and then with the above bug fix an invalid URL is generated: https://example.comuser/login (no slash at all). This applies to all subrequests - REQUEST_URI should always start with a slash as far as I can see, but BackendApiRequest currently strips it off.

    I'll push some changes that fix both these bugs.

  • Status changed to Needs review about 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK
  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    Forms generated in the frontend via the /ce-api route incorrectly remove the first slash. So /ce-api/user/login on becomes just user/login in the form action - and then with the above bug fix an invalid URL is generated: https://example.comuser/login (no slash at all). This applies to all subrequests - REQUEST_URI should always start with a slash as far as I can see, but BackendApiRequest currently strips it off.

    Thanks for catching this, good find!

    So the NativeCustomElementsFormController class is confusing me now - it seems to wrap the form in lupus-form like the regular form-controllers we provide, so what's the point? could we just go with CustomElementsEntityFormController instead?

    What really enables the "native" submission is fixing the form action to be absolute, but this is not part of this class but custom in user module.

    It would be great to add support for native form submissions in general and progressively take over form handling with client-side javascript when available / useful. But that seems to require a nodejs server processing the form so it the html can correctly reload in case form validation fails. So stuff for another issue.

  • Status changed to Needs work about 1 year ago
  • πŸ‡¦πŸ‡ΉAustria fago Vienna
  • πŸ‡¬πŸ‡§United Kingdom Dan.Ashdown

    Dan.Ashdown β†’ made their first commit to this issue’s fork.

  • Status changed to Needs review 8 months ago
  • πŸ‡¬πŸ‡§United Kingdom Dan.Ashdown
  • πŸ‡¬πŸ‡§United Kingdom Dan.Ashdown
  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    Checking this again, I'm still wondering about that:

    So the NativeCustomElementsFormController class is confusing me now - it seems to wrap the form in lupus-form like the regular form-controllers we provide, so what's the point? could we just go with CustomElementsEntityFormController instead?
    

    What makes a native form a native form?

    We could modify it inside CustomElementsFormController::getContentResult() but this feels like the wrong place, and is coupling the changes to the form controller, whereas we only really want it in the user_form module.

    This seems to be right place to me, so NativeCustomElementsFormController:getContentResult() could make sure the URL is absolute and form submission starts working natively that way. If we could also solve the redirect to be triggered within NativeCustomElementsFormController one could add support for native-forms simply by adding in the right routes with NativeCustomElementsFormController. That would be a quite good DX imho.

    But then on the frontend I think we should differ between vue-js handled form submissions and natively working forms which need no special handling at all. What do you think about doing custom elements with the names:
    - the native one
    - the one rendering into custom-elements, so the frontend has to take care of processing it

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    Giving this topic a bit more though, I think we need to consider how forms should be handled progressively first, because that will influence what we need here.

    So for progressive form submissions, I think we want to the backend render the forms as usual and just make it so that the form-markup gets shipped into a drupal-form custom element. We pass the form content as nested content slot, and all the regular
    attributes as props.

    Then, the frontend can easily render the form while controlling necessary attributes like the form action. Usually, by default, we just point the form action against the same route, like drupal does. So a POST to the same route is forwarded by the nuxt-server to the backend, where the form submission is processed as usual. If it results in a redirect with messages, the server will handle this (like we do for every page request). If it results in a reloaded form it can be rendered as part of the page as usual, optionally with messages. So the progressive submission should work quite straight-forward.

    Then, the frontend can handle more case, thanks to controlling the form element:
    - The frontend can add event handlers for handling form submissions with JavaScript. The JavaScript needs to post the data to the Drupal route of the page and handle the response. We might be able to leverage Nuxt server components elegantly for that, which would re-render server side when updated. We can explore something like a server-only drupal-form-handler component for that, which is used by the wrapping element.
    - When doing static generation, the frontend can take care of re-fetching the form and form-build ID as needed. We could provide a composable for that. Then the form is fully working based on JavaScript-enhanced submissions.
    - Static generation won't have working progressive form submissions, unless pointed directly to the Drupal backend. If that's good enough the frontend can do so for some forms by making the form action absolute. of course form validation errors would not be handled properly.

    So what does that mean for the backend:

    • For progress form submissions, we should support re-rendering the whole page when form data is posted.
    • For handling JavaScript enhanced form submissions, we should support only rendering the form as part of the response. Alternatively, the frontend would have to cut-out the relevant part of the response.

    Given that, I don't thank we should add forms with absolute form actions on the backend, but focus on rendering forms within the CE-page-response in a wrapper. That will allow use to make progressive forms work quite easily, for all kind of drupal forms, plus enable us to add JavaScript-enhanced submissions in a following step.

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    I've merged into 1.x

    Re-checking on how to solve this, some thoughts:
    * We need to have a simple way to wrap a built $form render array into a CustomElement drupal-form
    * Would be nice to add a FormRouteEnhancer that automatically adds in CustomElementsFormController if _format is custom_elements
    * Then backend form rendering into a CE-enabled API response should be straight forward and progress form submissions in SSR should mostly just work
    * Once that is working, JavaScript enhanced form submissions are less a priority for SSR sites, more for SSG sites. That can be handled in a follow-up then.
    * As demo forms we should have contact forms and user login forms sub-modules

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    Some thoughts:
    * We should check whether it would be enough to provide one CustomElementsFormController and a trait. less loptions, less confusion.
    * Most logic seems to boil down to trait method

      public function getCustomElementsContentResult(array $form): CustomElement {
        return CustomElement::createFromRenderArray($form)
          ->setTag('lupus-form');
      }
    

    that we want to improve to render into a "drupal-form" component, which gets the data to render the
    element, so it can gain some control over the form.
    * By default, we should submit forms via the proxy, back to drupal via /ce-api prefix of the page. So when the route is ce-enabled, the form should be processed as ususal. Validation fails should work transparently that way. For that to work, we need to make sure the frontend correctly passes through POST requests. That should do away the need for all that custom-code for the user forms.
    * For adding support for a form, a controllers needs to be provided which renders the form and creates the drupal-form CE via the helper like getCustomElementsContentResult() for the result. If form processing results in a redirect, lupus-ce-render should be able to handle that transparently.

  • πŸ‡¦πŸ‡ΉAustria fago Vienna

    I pushed my backend code at 3336148-forms-simplified - it's largely simplified. I need to add the docs still though.

Production build 0.69.0 2024