Layout Builder "Add Section" does not have submit button

Created on 20 March 2024, 2 months ago
Updated 8 April 2024, about 2 months ago

Problem/Motivation

In LB + JSX Umami, the "Add Section" dialog works, and it is possible choose a section and advance to the next form where the admin label and column dimensions can be set. However, there is no submit button showing up. Lets fix that here. If within this issue, the submit button does not work despite showing up, that can be moved to a followup OR it can be addressed in this issue - whatever scope seems to work best.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

๐Ÿ› Bug report
Status

Needs work

Version

1.0

Component

Umami JSX Demonstration Theme

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

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

Merge Requests

Comments & Activities

  • Issue created by @bnjmnm
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    yash.rode โ†’ made their first commit to this issueโ€™s fork.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    yash.rode โ†’ changed the visibility of the branch 3432283-layout-builder-add to hidden.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    yash.rode โ†’ changed the visibility of the branch 3432283-layout-builder-add to active.

  • Pipeline finished with Success
    2 months ago
    Total: 1028s
    #125912
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    The changes create a condition that is always false - that will need updating which may just mean removing the condition and the effect entirely, see MR comment for more.

  • Pipeline finished with Failed
    2 months ago
    Total: 1215s
    #129259
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Removed the

    if (childNode.hasAttribute('data-drupal-scriptified')) {
              break;
            }

    blcok, which solves this issue, but fails tests. As well as when tried with the Ajax test module, some of the functionality does not work.
    example: when we open the Link 5 (form) on ajax-test/dialog page-- and then click on the Hello world button we expect a Hello world status message, but it does not work.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    blcok, which solves this issue, but fails tests. As well as when tried with the Ajax test module, some of the functionality does not work.
    example: when we open the Link 5 (form) on ajax-test/dialog page-- and then click on the Hello world button we expect a Hello world status message, but it does not work.

    Sounds like it's time to work on a solution that both fixes the problem reported here and allows scenarios such as clicking Link 5 to work ajax-test/dialog. You're already more than halfway there because you know it has something to do with removing this block of code

    if (childNode.hasAttribute('data-drupal-scriptified')) {
              break;
            }
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Hi @bnjmnm, I tried debugging this, I think there is something wrong with Drupal.behaviors.AJAX's attach method, first thing we need to change the timeout to 50000000000ms(lesser than this might also work but 50 doesn't work) so that the element get's available and we can add `drupal-ajax` to it.
    I was able to add the `drupal-ajax` but still the functionality is still not working, I further went on debugging ajax.modified.js's Drupal.ajax but it is working same as non-jsx. So I am not sure what's missing in there. It would be helpful, If you could take a look and let me know what could be the next step on this.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    Nice investigating!

    This is new territory so the following suggestion might not directly be applicable to whatever the eventual solution is, but even in the worst-case it will familiarize you with concepts that will benefit your work on the JSX Theme Engine overall.

    I suspect this is roughly what is occurring:

    • On initial page load AND anytime the DOM is modified via ajax, Drupal.attachBehaviors is called, which executes every Drupal.behavior and the context is either the whole page (on initial load) or the part of the DOM altered by the AJAX call.
    • Behaviors that are triggered on initial page load happen as soon as the markup is rendered... but BEFORE the JSX is rendered.
    • Some behaviors use once() or other means of not re-running the same logic based on adding attributes to an element in the DOM. However, if these once-only operations are provided on the pre-JSX-rendered version of these elements, they may wind up rendered on the page with attributes that indicate they've received the proper JavaScripting... but they are not the same elements.
    • I can think of two general ways to getting around this:
      1. When the JSX theme engine is in use, delay the initial dom-ready call to Drupal.attachBehaviors and have it run once the initial JSX render occurs. Perhaps an event can be triggered when the JSX initial render happens, and THEN the initial Drupal.attachBehaviors calls happen. This is probably the best approach as it will likely fix many other issues we're not aware of... but it's also a large change with potential side effects so the other option to consider is...
      2. During the JSX render process, strip out any attributes that make the Javascripting happen only once, as these are new elements in the DOM that will need to be processed by behaviors.

    That's a whole lotta info above, so think of this more as something to guide your exploration into this issue vs something that has to fully click as soon as you read it ๐Ÿ™‚. We're messing with some fundamental parts of Drupal so it'll be weird but hopefully interesting at the same time.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Hi, I am trying to implement

    When the JSX theme engine is in use, delay the initial dom-ready call to Drupal.attachBehaviors and have it run once the initial JSX render occurs. Perhaps an event ca...

    I need help with that,
    we want to dispatch an event at the end of index-react.js::processJSX with some timeout, so that all the React elements gets rendered and then we want to call the Drupal.attachBehaviors in drupal.init.js once the event is triggered from the index-react.js::processJSX? Is this what you are trying to say in previous comment?

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    Something to try:
    In the top-level component on the first render (I believe this is the DrupalContextApp component) add a componentDidMount which will be invoked as soon as that first render completes. Pop an event dispatch in there to let the world know things are rendered and awaiting further instruction

     componentDidMount = () => {
        window.dispatchEvent(new Event('first-render'));
      }

    Then in an override of drupal.init.js, switch the domReady to something that responds to the event dispatched on first render

      // NOT THIS ๐Ÿ‘‡ , IT DOES NOT KNOW HOW TO HANDLE REACT.
      // domReady(() => {
      //    Drupal.attachBehaviors(document, drupalSettings);
      // });
    
      // BUT THIS ๐Ÿ‘‡is effectively reactReady instead of 
      // domReady()
      window.addEventListener('first-render', () => {
        Drupal.attachBehaviors(document, drupalSettings);
      })

    I believe this (or something like this) will address what was specifically mentioned in #12 - the need to introduce a huge timeout to Drupal AJAX attach. That delay was good troubleshooting because it demonstrated that the behavior was executing too soon, and I believe the specific too-soon is that it was happening before the React render. With the approach mentioned above this should postpone that initial behavior invocation until the dom is Reactified.

  • Pipeline finished with Failed
    2 months ago
    Total: 1330s
    #132201
  • Pipeline finished with Failed
    2 months ago
    Total: 1269s
    #132242
  • After trying what's described in #15 โ†’ and also adding timeout, we were able to get some parts of the modal form work,but the problem in this is that only 2 buttons are working(hello world and do it) and the preview button is not working as expected(after clicking this button the form is not loading in a modal).
    In successive clicking of any buttons it loads the wrong form i.e the form that should load on clicking preview button and also the forms are not opening in the modals.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    Two spots I recommend looking at:

    Our version of the dialog.ajax functionality

    This is 98% the same as Drupal core's dialog.ajax.js, and all the changes thus far are specific to dialog + buttons functionality. Specifically, to prevent duplicate buttons.
    You'll want to see how prepareDialogButtons($dialog) works, and when / where it is called in order to determine why some buttons are failing and others work. This is the method that takes buttons from the form-in-a-dialog and moves them to the dialog's buttonpane. Setting some debug points to identify what is different about preview should help you better understand what is happening and where the point of failure is.

    In the added AJAX logic

    Currently the nested components do not get pushed through behaviors.. I'm kind of surprised as much works as it does without this being the case. It's possible there could be problems if the buttons are in a context that doesn't have a component-element as the top parent. This is all new territory so I can't guarantee this is where to look, but it's definitely worth investigating.
    Note the @todo

             [...context.querySelectorAll(`${componentName}:not([data-drupal-scriptified])`)].forEach(component => {
               if (!component.hasAttribute('data-drupal-scriptified')) {
                 const container =  Drupal.JSXAdditional(Drupal.JSXHyperscriptify(component),component);
                 component.setAttribute('data-drupal-scriptified', true)
                 component.hidden = true;
                 // @todo: it is probably necessary to have a Drupal.attachBehaviors(container, {...settings, doNotReinvoke: true});
                 // here so it isn't only happening when the top-level parent of context is a JSX component.
                 // It isn't here currently as it was causing duplicate renders. It is possible this is no longer
                 // a problem due to fixes made elsewhere.
               }
             })
    
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    While looking at prepareDialogButtons($dialog)
    Looking at the function which is calling prepareDialogButtons is same as the core's but, Drupal.AjaxCommands.prototype.openDialog get's only one button when we try to call prepareDialogButtons($dialog). We should get all the three buttons just like non-jsx.

    but in non-jsx, we are getting all the three button in

    if (
          !response.dialogOptions.buttons &&
          response.dialogOptions.drupalAutoButtons
        )

    and in our case there we get no buttons in this if and getting only one(preview) button in the next call to const buttons = Drupal.behaviors.dialog.prepareDialogButtons($dialog);. So, Is that a expected behaviour or a there is something wrong?

    We also added Drupal.attachBehaviors for the nested components, but it does not make any difference as of now.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    Based on a Zoom call, it looks like implementing the changes that allow this submit button to appear also result in AJAX-enabled dialog buttons not being AJAXified. We were able to narrow it down to this:
    When a form is inside a dialog, any buttons in actions are moved from the <form> to the dialog's buttonpane

    If any of those buttons have Drupal-AJAX functionality, it is implemented in Drupal.behaviors.AJAX via the loadAjaxBehavior function

       function loadAjaxBehavior(base) {
            const elementSettings = settings.ajax[base];
            if (typeof elementSettings.selector === 'undefined') {
              elementSettings.selector = `#${base}`;
            }
            // Use jQuery selector instead of a native selector for
            // backwards compatibility.
            // Add timeout to provide time for scriptified elements.
            setTimeout(() => {
              once('drupal-ajax', $(elementSettings.selector)).forEach((el) => {
                elementSettings.element = el;
                elementSettings.base = base;
                Drupal.ajax(elementSettings);
              });
            }, 50);
          }

    As the code above demonstrates, the element that should get AJAX functionality is found via elementSettings.selector. In the case of these buttons, elementSettings.selector is finding two matches - one is the visible element that is part of the page, and the other is inside a <drupal-fragment> as the not-visible markup that hyperscriptify() then renders into something interactable. In the case of these AJAX dialog forms (and probably elsewhere) the selector is finding the version inside <drupal-fragment> first, and applying the AJAX functionality to an element that is not part of the rendered page.

    This also means that duplicate ids can exist on the page in general, existing in the hyperscriptify-informing markup as well as the final rendered page.

    The best way to address this isn't immediately clear, but here are some approaches that come to mind:

    • Update queries to ignore elements inside drupal-fragment and other custom elements.
    • Alter the IDs of elements inside drupal-fragment and other custom elements, and change them back to the expected ids when they are hyperscriptified (this could fix the duplicate id problem too)
    • Update the prevent-duplicate-ids script to prioritize changing ids inside <drupal-fragment> etc when two instances are found and only one is for actual eyeballs.
    • Find a way to ensure that the contents inside <drupal-fragment> etc are removed from the DOM before their visible-to-people hyperscriptified versions are sent through attachBehaviors()
  • Pipeline finished with Failed
    2 months ago
    Total: 1311s
    #135151
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Update queries to ignore elements inside drupal-fragment and other custom elements.

    Tried this, still Hello world button does not work.

  • Pipeline finished with Failed
    2 months ago
    Total: 1480s
    #135398
  • Pipeline finished with Failed
    2 months ago
    Total: 1432s
    #135659
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1025s
    #135912
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1453s
    #136054
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1429s
    #136335
  • So we tried to implement

    Alter the IDs of elements inside drupal-fragment and other custom elements, and change them back to the expected ids when they are hyperscriptified (this could fix the duplicate id problem too)

    (last 2 commits).But the problem with this is Hello world has stopped working.Is the setting and resetting id's done at correct place?It would be great if you can verify the the changes.
    Before the commits related to the changes of id all the 3 buttons were working as expected except the fact that if we click on Do it and then open the same modal(link5) it does not open it in a modal.
    So what's the thing that i should prioritise from the above two.Shall I try to fix the Do it behaviour or shall I try to get the id's replaced correctly.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    Lets table the id approach for now. There's yet another approach that is worth trying out:

    You'll first want to refactor every call to Drupal.attachBehaviors() in ajax.modified.js. If the first arg is a child of (or is) [data-off-canvas-main-canvas]</code, that arg should be <a href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template">wrapped in a <code><template> tag. Template contents are not directly queryable, so we don't have to worry about duplicate ids or behaviors data-onceing an element that is not meant to be part of the rendered page.

    Inside Drupal.behaviors.jsxAjaxProcess.attach() check to see if context is a template tag. Exit early if it isn't (you may want to keep the duplicate id prevention, but everything else can exit early.

    It even may be possible to just send the <template>wrapped context without all the additional checks - so if it's a template go
    Drupal.JSXAdditional(Drupal.JSXHyperscriptify(context), context);. If that results in nothing appearing you may need to send context.contentinstead.

    By wrapping the code in <template>, it is shielded by DOM queries. Once it is scriptified, though, it can be sent to behaviors and AJAXed properly.

    How to exactly get this approach to work might require some trial and error. It'll be easier to prgress the more you familiarize yourself with how template works on MDN and the docs at the beginning of demo/themes/umami_jsx/app/local_packages/hyperscriptify/index.js

  • So i tried digging into the steps provided in #22. With

    You'll first want to refactor every call to Drupal.attachBehaviors() in ajax.modified.js. If the first arg is a child of (or is) [data-off-canvas-main-canvas]wrapped in a tag. Template contents are not directly queryable, so we don't have to worry about duplicate ids or behaviors data-onceing an element that is not meant to be part of the rendered page.

    changes applied all the 3 buttons are working correctly now.
    But on subsequent clicking of the link5 it sometimes opens in a modal and sometimes it does not.I will try to work on the next suggestions.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 1391s
    #137141
  • With the latest changes if we click on Hello world or Preview button then subsequently the form always open in a modal and all the buttons works as expected.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 1383s
    #137193
  • One thing that i noticed is that after we click the preview button it adds form-actions class

    Do itPreviewHello world

    to this div.Can this be the problem that the form is not loading in modal?

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    When I bring down the current branch I see a console error with bindAjaxLinks in the trace, and bindAjaxLinks is what alters link behavior to open a modal instead of the default behavior of going to another page - addressing that error will likely address issues where clicking is failing to execute the expected AJAX task

  • Pipeline finished with Canceled
    about 2 months ago
    Total: 468s
    #138250
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1447s
    #138253
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1438s
    #138828
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Hi, I tried the current state of MR on my local, it is working for the test modal page but it is not working for the layout builder Add section page.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 936s
    #140547
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 60s
    #140582
  • Pipeline finished with Canceled
    about 2 months ago
    #140583
  • Pipeline finished with Running
    about 2 months ago
    #140589
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1439s
    #140649
  • Pipeline finished with Failed
    about 2 months ago
    Total: 2611s
    #140696
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1407s
    #141285
Production build 0.69.0 2024