vaadin / framework

Vaadin 6, 7, 8 is a Java framework for modern Java web applications.
http://vaadin.com/
Other
1.77k stars 729 forks source link

v8 Grid field focus breaks frozen columns #12560

Closed softcomarkusheikkinen closed 1 year ago

softcomarkusheikkinen commented 1 year ago

Vaadin 8.17.0 and Vaadin 8 Grid.

Three issues on grid with frozen columns and editable fields after frozen columns. Field value change triggers focus to other field e.g. using ValueChangeMode.LAZY.

  1. Tab navigation in fields after frozen column flickers the grid. Just navigate in grid fields using tab.

  2. Frozen columns breaks when focus is moved to field that is not completely visible. Resize view so that second field is not visible (see initial state image) . Type value to first field and wait value change (don't use tab), frozen columns are overlapped with other columns

Initial state image After value change + focus from server size image

  1. When focus is moved from field to previous field that is not visible, grid does not scroll field to be visible. Resize view so that first field is not visible when second field is scrolled visible. Type value to second field and wait value change (don't use tab), focus is moved to field. but not scrolled to view.

Initial state image After value change + focus from server size image

Attached simple test application used to reproduce issues. Can be run using maven jetty:run. grid-focus.zip

Ansku commented 1 year ago

I've been poking around this for a few hours, and I suspect that there isn't much that can be done about 1. -- both tabulator navigation and autoscrolling the element to view are done by the browser itself, and even if the key event were to be captured in order to block the native handling, trying to reimplement the desirable parts for a generic Grid would be a scary proposition indeed. Current Grid implementation already does the thing that can be done with a reasonable effort, i.e. immediately moves back the elements that should not have been moved by the autoscrolling, which unfortunately still leaves the flickering. Overriding the tabulator navigation for a specific Grid could be doable, but there is no easy solution or even workaround on the framework level.

The 2. and 3. are somewhat easier to work around, since both problems go away if the target column is scrolled into view before focusing the component. Unfortunately scrollToColumn method only exists in Escalator at the moment, although TreeGrid can get to the approximately same results with something like

    public class CustomTreeGrid extends TreeGrid<GridRow> {
        public void focusCell(int rowIndex, int columnIndex) {
            getRpcProxy(FocusRpc.class).focusCell(rowIndex, columnIndex);
        }
    }

which would turn the value changes to something like

            textField1.addValueChangeListener(e -> {
                grid.focusCell(rowIndex, columnIndex);
                textField2.focus();
            });

Sadly regular Grid doesn't use FocusRpc, so there the same workaround would require also extending the GridConnector and using the violator pattern to access the private method in Grid widget, e.g. like this:

    public native void focusCell(Grid<JsonObject> grid, int rowIndex,
            int columnIndex)
    /*-{
        grid.@com.vaadin.client.widgets.Grid::focusCell(II)(rowIndex, columnIndex);
    }-*/;

and

        registerRpc(FocusRpc.class, (rowIndex,
                cellIndex) -> focusCell(getWidget(), rowIndex, cellIndex));

Of course Escalator's scrollToColumn is public even to begin with, so that one could be called without resorting to JSNI, but there is quite a bit of logic in that Grid method and I didn't have time to check yet what all might break if it is simply bypassed.

This same approach could also be used as a starting point for that custom tabulator handling for a specific Grid I mentioned earlier, but a whole lot of other handling would be required on top of that as well.

I'll discuss this further with the team next week to see if at least the column scrolling could be made easier.

softcomarkusheikkinen commented 1 year ago

I've tried the work-around by extending GridConnector & Grid with examples above. This works quite as expected when focus change is triggered by value change or moved by mouse. Frozen columns will still break by following changes

  1. Move focus to first field, type value and wait (focus is moved to next field)
  2. Move focus back to first field by shift + tab
  3. Type and wait -> frozen columns are broken as in original issue
Ansku commented 1 year ago

This has proven surprisingly tricky to debug, but there are indeed issues in the element position handling that creep up in some but not all use cases using the workaround I listed above, depending on what exactly updates those positions and in which situations. I think I've found an ugly brute force solution that can deal with the variety of ways I've been testing lately, but unfortunately it's a bit of a hiding-the-symptoms sort of approach. I'm still hoping to find something a bit more elegant that wouldn't have to do quite so many calculations quite so often.