Make Views AJAX scroll to top optional

Fixed path in the patch

Fixed a typo in a comment

#4 patch works for me on 10.1.1. This is a small but necessary option.

This is great - thank you @skyredwang

That's cool, thanks @skyreddwang,
Tested on D9.5.8 /PHP 8.1/MariaDB 10.3

Hi @skyredwang
I applied the patch #4. Works well.
Thanks

I think it's reviewed and tested by community, so I think we can change status of this issue

Thanks for working on this, the patch should be moved to a Merge Request so that tests can run. Talking about tests, a test should be added per #7 to make sure we don't break that feature down the line.

Thanks!

Hi ,

I tried to reproduce the issue issue is replaceable in 11. x.

And I created the MR for 11. x and tests are passed.

RTBC++

Please let me know if I missed anything happy to take suggestions.

Thank You!!

Previously tagged for tests which are still needed.

Did not review.

Added unit test to prove new functionality.

Attaching patch simply for composer.

So I imagine with this config change we will need a schema update for the new setting.

Existing view config will have to probably be updated.

Also imagine there will have to be a post_update hook to set the new key for existing views (should be batch)

A unit test is in place. Test-only test fails as it should. Removing 'Needs tests' tag. If further test coverage is needed, please update.

+1 for this addition to core. In the meantime, you can also accomplish this with an event subscriber:

namespace Drupal\my_module\EventSubscriber;

use Drupal\views\Ajax\ViewAjaxResponse;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Change the scroll-to-top behavior for the my_view view.
 */
class AjaxResponseEventSubscriber implements EventSubscriberInterface {

  /**
   * {@inheritdoc}
   *
   * @return array
   *   The event names to listen for, and the methods that should be executed.
   */
  public static function getSubscribedEvents() {
    return [
      KernelEvents::RESPONSE => 'alterCommands',
    ];
  }

  /**
   * React to ResponseEvent.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   Response event.
   */
  public function alterCommands(ResponseEvent $event) {
    $response = $event->getResponse();

    if ($response instanceof ViewAjaxResponse
      && $response->getView()->id() === 'my_view'
      ) {

      $commands = &$response->getCommands();
      foreach($commands as $key => $command) {
        if ($command['command'] == 'scrollTop') {
          unset($commands[$key]);
        }
      }
      $commands = array_values($commands);
    }
  }

}

In my case, I wanted to modify the selector to the views' .view-content class, which is only a minor change:

      $commands = &$response->getCommands();
      foreach($commands as $key => $command) {
        if ($command['command'] == 'scrollTop') {
-         unset($commands[$key]);
+         $commands[$key]['selector'] = $command['selector'] . ' .view-content';
        }
      }
-     $commands = array_values($commands);
    }

It'd be worthwhile to consider allowing a site builder to set the scrollTop selector instead of adding a toggle. It would meet my use case, which is necessitated by responsive design (where the exposed filters stack atop the content on mobile).

Reviewed and tested patch #4 for several month on D10 and it's working fine.
Well done! Thanks!

#19 still applies

Thanks all. I added the schema and while doing that think I spotted an issue: IIUC in \Drupal\views\Plugin\views\display\DisplayPluginBase::defineOptions() instead of providing a default value, we marked it as non-defaultable (ie it can't be reverted to default). I've updated it so that it is defaultable and also set the default value to off.

It still needs a config update to be written (more details in #3447755: Document ViewsConfigUpdater) and presumably existing core views to be updated.

Not a views expert, but it looks to me like we should also add a method to \Drupal\views\Plugin\views\display\DisplayPluginInterface. I was about to do that when I (at the huge risk of bike-shedding) wondered if it doesn't make more sense to have a scroll_to_top option that is set to enabled by default, rather than a disable_scroll_to_top. This just avoids things like the double negative of if (!$view->display_handler->getOption('disable_scroll_to_top')). Thanks!

+1
I my case this "scroll to top" was annoying. I was verry pleased there was a solution! Thanks!
I hope this is going to land in Drupal core.
Thanks,
Marcel

I have rebased the PR to address the PHPCS and PHPStan pipeline issues. All tests are passing in CI.
The current failure appears to be related to the JUnit artifact upload limits (phpunit-*.xml), not an actual test failure.