Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8213540

TableViewSkin: move focus with left/right keys must scroll to focused cell

    XMLWordPrintable

Details

    Description

      to reproduce, run the example below and make sure the last column is not completely visible:

      - ctrl click into any cell in first column to focus it (just focus, no selection)
      - ctrl-left to move focus until reaching last cell
      - expected: last column fully visible, that is scrolled into the viewport
      - actual: last column not completely visible, not scroll happened

      Technically, the reason is that onFocusNext/PreviousCell doesn't implement any scrolling.

      On a deeper level, the reason is incomplete api in TableViewSkinBase and TableViewBehaviorBase:

      - TableViewSkinBase is missing callback methods onFocusLeft/RightCell
      - TableViewBehaviorBase is missing setters for those missing callbacks, setOnFocusLeft/RightCell(Runnable)
      Adding those would be consistent with the set/onSelectXX api

      The missing methods (and their correct implementation) could be added in the related issue JDK-8207942

      For now, a dirty hack (which might have side-effects for pure up/down navigation, didn't test) is to re-implement (c&p) equivalents to onFocusPrevious/NextCell which additionally trigger scrolling to the cell and hook those into the callback slots of the behaviour (accessible only via reflection).

      Note: apart from being incomplete, the naming of skin's onXX methods are inconsistent compared to both the underlying models as well as compared to the corresponding behavior methods (also see Samir's pull request for TableViewSkinBase and a comparison of api https://github.com/kleopatra/swingempire-fx/wiki/TableSelectionModel). This could/should be fixed before exposing to protected/public scope.

      The example:

      public class TableViewScrollHorizontalBug extends Application {

          private Parent getContent() {
              TableView<Locale> table = new TableView<>(FXCollections.observableArrayList(
                      Locale.getAvailableLocales()));
              table.getSelectionModel().setCellSelectionEnabled(true);
              TableColumn<Locale, String> countryCode = new TableColumn<>("CountryCode");
              countryCode.setCellValueFactory(new PropertyValueFactory<>("country"));
              TableColumn<Locale, String> language = new TableColumn<>("Language");
              language.setCellValueFactory(new PropertyValueFactory<>("language"));
              TableColumn<Locale, String> variant = new TableColumn<>("Variant");
              variant.setCellValueFactory(new PropertyValueFactory<>("variant"));
              TableColumn<Locale, String> display = new TableColumn<>("DisplayName");
              display.setCellValueFactory(new PropertyValueFactory<>("displayLanguage"));
              table.getColumns().addAll(display, countryCode, language, variant);

              BorderPane pane = new BorderPane(table);
              return pane;
          }

          @Override
          public void start(Stage primaryStage) throws Exception {
              primaryStage.setScene(new Scene(getContent(), 200, 400));
              primaryStage.setTitle(FXUtils.version());
              primaryStage.show();
          }
          
          public static void main(String[] args) {
              launch(args);
          }
          
          @SuppressWarnings("unused")
          private static final Logger LOG = Logger
                  .getLogger(TableViewScrollHorizontalBug.class.getName());

      }


      Attachments

        Issue Links

          Activity

            People

              arapte Ambarish Rapte
              fastegal Jeanette Winzenburg
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated: