#3508969: If any errors happen during activation, automatically scroll to the messages area
Closes #3508969
Merge request reports
Activity
added 1 commit
- 922a3137 - Instead of strict checking test the range of Y coordidnate
106 106 $message = 'Unable to install modules pinky_brain due to missing modules pinky_brain'; 107 107 $this->assertPageHasText($message); 108 108 $this->assertSession()->statusMessageContains($message, 'error'); 109 // Assert that the page has scrolled to the top. 110 $scroll_position = $this->getSession()->evaluateScript('return window.scrollY;'); 111 // This does not scroll to exact top so testing it in a range of Y coordinate. 112 $this->assertLessThanOrEqual(250, $scroll_position); - Comment on lines +109 to +112
I think this is a really clever approach, but I think it could also pass even if the command didn't work, because if the test never scrolled below 250 pixels to begin with, then this would pass anyway.
That said, there's a way to find out. If you go to https://git.drupalcode.org/issue/project_browser-3508969/-/pipelines/435554 and run the "test-only changes" jobs, those will run this changed test without the bug fix, and if those tests fail, then we'll know that this bug is properly fixed and the tests are really testing it. I don't have permissions to kick the job off, can you do the honors?
-
Another thing we could do here is assert that the current scroll position is greater than 0. That would prove that we scrolled somewhere that wasn't the top of the page (which would make sense, since nothing else in this test is scrolling around, as far as I know), and then were scrolled to a different place (presumably the messages area) when the error happened. So it would be something like this:
// Do something to ensure the messages block is placed at the _bottom_ of the region, so that we KNOW its Y position will be greater than 0. Not 100% sure how the Drupal.messages() JS works, but I think it will look for an existing messages block? $button = $this->waitForProject('Pinky and the Brain') ->findButton('Install Pinky and the Brain'); $this->assertSame(0, $get_scroll_position()); // ... // Install, check for the error, same as now // ... $this->assertGreaterThan(0, $get_scroll_position()); changed this line in version 6 of the diff
467 468 'type' => MessengerInterface::TYPE_ERROR, 468 469 ], 469 470 )); 471 $response->addCommand(new ScrollTopCommand('.messages__wrapper')); changed this line in version 4 of the diff
added 8 commits
-
feb0ae8e...7401da78 - 7 commits from branch
project:2.0.x - a7ac1750 - Merge branch '2.0.x' into 3508969-if-any-errors
-
feb0ae8e...7401da78 - 7 commits from branch
106 107 $message = 'Unable to install modules pinky_brain due to missing modules pinky_brain'; 107 108 $this->assertPageHasText($message); 108 109 $this->assertSession()->statusMessageContains($message, 'error'); 110 // Assert that the page has scrolled to the top. 111 $after_scroll_position = $this->getSession()->evaluateScript('return window.scrollY;'); 112 // This does not scroll to exact top so testing it in a range of Y coordinate. 113 $this->assertLessThan($after_scroll_position, $initial_scroll_position); -
@phenaproxima does this makes sense?
changed this line in version 7 of the diff
-
Well, this certainly proves that we scrolled upwards. It doesn't necessarily prove that we scrolled to the messages area.
I think we can probably be a bit more precise, and imprecise, at the same time -- what if we checked that the messages area was in the viewport, regardless of whether we actually scrolled to it?
added 2 commits
added 2 commits
added 3 commits
added 3 commits
-
896fd2c8...3848d355 - 2 commits from branch
project:2.0.x - d7298f07 - Merge branch '2.0.x' into 3508969-if-any-errors
-
896fd2c8...3848d355 - 2 commits from branch