Skip to content
Snippets Groups Projects

Issue #3321255: Only reload updated part of Layout Builder element

Open Issue #3321255: Only reload updated part of Layout Builder element
1 unresolved thread
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
157 $this->waitForOffCanvasArea();
158 $off_canvas = $this->assertSession()->elementExists('css', '#drupal-off-canvas');
159 $this->assertNotNull($off_canvas);
160 $form_id_element = $off_canvas->find('hidden_field_selector', ['hidden_field', 'form_id']);
161 // Ensure the form ID has the correct value and that the form is visible.
162 $this->assertNotEmpty($form_id_element);
163 $this->assertSame($expected_form_id, $form_id_element->getValue());
164 $this->assertTrue($form_id_element->getParent()->isVisible());
165 }
166
167 /**
168 * {@inheritdoc}
169 *
170 * @todo Remove this in https://www.drupal.org/project/drupal/issues/2918718.
171 */
172 protected function clickContextualLink($selector, $link_locator, $force_visible = TRUE) {
  • Was this copied from somewhere else? Should we make a trait for it to avoid duplicating?

  • This is copied. I'm not sure if I was aware of the trait that already exists. It looks like there might be issues with the trait being fully functional in this context? The linked issue about it appears to still be open. I'm up for whatever, just want to make sure I'm thinking about it correctly.

  • So that would mean we have the original (broken) trait with the todo to fix it, plus two copied fixed versions in LB. Can we combine the two in LB into a new trait perhaps?

  • @larowlan In looking at this a little bit closer, Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderTest (the test I referenced when creating the new test in this MR) uses Drupal\Tests\layout_builder\FunctionalJavascript\ContextualLinkClickTrait and overrides the clickContextualLink method from that trait. I think the reasoning behind this is that trait's clickContextualLink method doesn't currently work correctly with LB. The overridden method in the test class can be removed whenever that issue is resolved and then it will just be using the method from the trait.

    If I create a new trait with this method that is LB-specific and replace the instances where the other trait are used, then it is slightly more complicated to switch to the existing trait later on. Instead of just deleting the two overridden methods, it would require updating the use statements in both test classes and deleting the trait. The existing trait also has another method that is not overridden, so if I created a new trait, would I be copying that over as well?

    If you would still like me to do this, I can do it. I just wanted to provide that context in case it influences the decision and get feedback about how to proceed.

    Edited by Sean Adams-Hiett
  • I think that sounds like premature optimisation and out of scope here. Can you open a followup issue to explore and add a todo in the code pointing to it?

  • Please register or sign in to reply
  • Ted Bowman added 575 commits

    added 575 commits

    Compare with previous version

  • Please register or sign in to reply
    Loading