Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scrollIntoView no longer works within Grid as of Vaadin 8.18.0 #12580

Closed
pogocode opened this issue Feb 28, 2023 · 7 comments
Closed

scrollIntoView no longer works within Grid as of Vaadin 8.18.0 #12580

pogocode opened this issue Feb 28, 2023 · 7 comments
Assignees
Labels
wontfix workaround Workaround provided in ticket comments

Comments

@pogocode
Copy link

pogocode commented Feb 28, 2023

As of Vaadin 8.18.0, it is no longer possible to use scrollIntoView to move cells within a grid.

Minimal Reproducible Example

The following application using Vaadin version 8.17.0 works correctly (i.e. scrollIntoView works):

mpr-demo-mpr-6-grid-scrolling-8-17-0.zip

The following application using Vaadin version 8.18.0 does not work correctly (i.e. scrollIntoView does not work):

mpr-demo-mpr-6-grid-scrolling-8-18-0.zip

For both of the above applications (which are the same with the exceptions of the Vaadin version)

Start the application:

mvn -Pproduction jetty:run

Navigate to the following:

http://localhost:8080/legacy

In the 8.17.0 version:

  • Click Scroll to last cell - the last cell in the grid is displayed
  • Click Scroll to first cell - the first cell in the grid is displayed

In the 8.18.0 version:

  • Click Scroll to last cell - nothing happens

Expected behaviour

It should be possible to use scrollIntoView within the grid.

Actual behaviour

As of version 8.18.0 it is no longer possible to use scrollIntoView to navigate within the grid.

Versions:

Vaadin Framework version (e.g. 8.3.1)
Browser version - Chrome version 110.0.5481.100 (Official Build) (x86_64)
Web container name and version - Jetty 9.4.14.v20181114

Also the following, although these are probably not relevant to the issue (it just happens that I work with MPR and this was the simplest way for me to present the issue:

Flow: 23.3.6
MPR: 6.1.1
Java JRE / JDK: 11

@Ansku Ansku self-assigned this Feb 28, 2023
@Ansku Ansku added the BFP label Feb 28, 2023
@Ansku
Copy link
Member

Ansku commented Feb 28, 2023

I can reproduce the issue, but why do you need the scrollIntoView? Wouldn't grid.scrollToColumn(grid.getColumns().get(9)) serve the same purpose with less hassle?

@Ansku
Copy link
Member

Ansku commented Feb 28, 2023

As a bit of a background, the functionality was changed in 8.18.0 because of #12560, and the scrollToColumn was also added then. That method has existed on the client side for a long time, but now it's easier to call it from the server too. I'm not entirely certain that those changes can coexist with the direct JavaScript call, but I can look into it if you have a use case that can't be solved by that new method or simply focusing a component within the target cell.

@pogocode
Copy link
Author

pogocode commented Feb 28, 2023 via email

@Ansku
Copy link
Member

Ansku commented Feb 28, 2023

Client-side Grid has scrollToColumn(int columnIndexDOM, ScrollDestination destination), and it calls Escalator scrollToColumn(final int columnIndex, final ScrollDestination destination, final int padding), where the default padding is destination == ScrollDestination.MIDDLE ? 0 : 10. I think it should be possible to call either of these methods directly.

@Ansku
Copy link
Member

Ansku commented Feb 28, 2023

I just realized I misread your comment. No, there is no built-in JavaScript method for custom calls for the client-side. With TestBench you can simply call $(GridElement.class).first().getCell(0, 9); to scroll the queried cell into view. I haven't tried that with pure Selenium, but I'd say those helper methods in TestBench exist for a good reason...

@pogocode
Copy link
Author

pogocode commented Feb 28, 2023 via email

@Ansku
Copy link
Member

Ansku commented Mar 1, 2023

As a quick-and-dirty workaround, you could also extend the GridConnector e.g. like this (with the JSNI call updated to match your package structure):

@SuppressWarnings("deprecation")
@Connect(com.vaadin.ui.Grid.class)
public class CustomGridConnector extends GridConnector {
    @Override
    protected void init() {
        super.init();
        exposeScrollToColumn(this, getWidget().getElement());
    }

    private void scrollToColumn(int index) {
        getWidget().scrollToColumn(index, ScrollDestination.MIDDLE);
    }

    private native void exposeScrollToColumn(GridConnector connector,
            Element grid)
    /*-{
        grid.scrollToColumnJS = $entry(function(index) {
            return connector.@org.vaadin.mprdemo.client.CustomGridConnector::scrollToColumn(*)(index);
        });
    }-*/;
}

and then call it with "document.getElementById('the-scrolling-grid').scrollToColumnJS(9)".

I haven't spent a whole lot of time thinking that through so it's not very polished, but it seems to work in my test project at least.

@Ansku Ansku added wontfix workaround Workaround provided in ticket comments labels Mar 2, 2023
@Ansku Ansku closed this as completed Mar 2, 2023
@Ansku Ansku closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2023
@Ansku Ansku removed the BFP label Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix workaround Workaround provided in ticket comments
Projects
None yet
Development

No branches or pull requests

2 participants