assertOffCanvasFormAfterWait() doesn't check for the correct form ID

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderTest.php
@@ -462,17 +462,20 @@ private function openAddBlockForm($block_title) {
-    $this->assertNotEmpty($page->waitFor($timeout / 1000, function () use ($page, $expected_form_id) {
-      // Ensure the form ID exists, is visible, and has the correct value.
-      $form_id_element = $page->find('hidden_field_selector', ['hidden_field', 'form_id']);
-
+    $form_id_element = $page->waitFor($timeout / 1000, function () use ($page) {

It seems like it would simpler to not use waitFor() at all.

We could just use

$off_canvas = $this->assertSession()->waitForElementVisible('css', '#drupal-off-canvas');
    $this->assertNotNull($off_canvas);

And then change the $form_id_element based off that.

Hi @tedbow

Made changes please review.

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderTest.php
@@ -469,17 +469,12 @@ private function openAddBlockForm($block_title) {
+    $this->assertSame($expected_form_id, $off_canvas->getValue());
+    $this->assertTrue($off_canvas->getParent()->isVisible());

$off_canvas is not for id element. We still need the logic from the previous patch to get that
$off_canvas->find('hidden_field_selector', ['hidden_field', 'form_id'])
'
And then basically the last 3 lines from the previous patch

1. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderTest.php
   @@ -463,23 +463,17 @@ private function openAddBlockForm($block_title) {
   -   * @param int $timeout
   -   *   (Optional) Timeout in milliseconds, defaults to 10000.
   +   *
       */
   -  private function assertOffCanvasFormAfterWait($expected_form_id, $timeout = 10000) {
   -    $page = $this->getSession()->getPage();
   +  private function assertOffCanvasFormAfterWait($expected_form_id) {
   

   Since this is private function and no caller was using $timeout we can remove this here 🎉

   Nit: needs work for the extra line at the end of the docblock

2. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderTest.php
   @@ -463,23 +463,17 @@ private function openAddBlockForm($block_title) {
   -    $this->assertNotEmpty($page->waitFor($timeout / 1000, function () use ($page, $expected_form_id) {
   ...
   +    $off_canvas = $this->assertSession()->waitForElementVisible('css', '#drupal-off-canvas');
   

   The actual wait time does not change because \Drupal\FunctionalJavascriptTests\JSWebAssert::waitForElementVisible() defaults to 10000 milliseconds.

3. While reviewing this patch I noticed that docblock for this function says

   Waits for the specified form and returns it when available and visible.

   But nothing was ever returned from this function and the callers never relied on return value.

   We should update this but I guess strictly speaking it should be another issue.

   I say we fix it in a follow up and don't fix in this issue but if I committer wants to say it should just be fixed here I would glad to have it done here.

Needs work for the nit in 1)

Here I have made changes as mentioned in comment #14

Thanks everyone! Looks good 🎉

Committed 700a6ab and pushed to 9.1.x. Thanks!

Backported to 8.9.x and 9.0.x