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

8129123: ComboBox popup list view does not scrollTo when ComboBox value/selection is set #166

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -417,8 +417,10 @@ private void updateValue() {
T itemsObj = comboBoxItems.get(index);
if ((itemsObj != null && itemsObj.equals(newValue)) || (itemsObj == null && newValue == null)) {
listViewSM.select(index);
listView.scrollTo(index);
} else {
listViewSM.select(newValue);
listView.scrollTo(newValue);
}
} else {
// just select the first instance of newValue in the list
@@ -429,6 +431,7 @@ private void updateValue() {
updateDisplayNode();
} else {
listViewSM.select(listViewIndex);
listView.scrollTo(listViewIndex);
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,8 @@

import test.com.sun.javafx.scene.control.infrastructure.KeyEventFirer;
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;

import javafx.scene.control.skin.ComboBoxListViewSkin;

import static test.com.sun.javafx.scene.control.infrastructure.ControlTestUtils.assertStyleClassContains;
@@ -2058,4 +2060,57 @@ public void test_rt_40012_selectedItemNotificationOnDisjointRemovesAbove() {

assertEquals("ComboBox skinProperty changed more than once, which is not expected.", 1, skinChangedCount);
}

@Test public void test_JDK_8129123() {
final int LIST_SIZE = 50;

ComboBox<String> comboBox = new ComboBox<>();

BorderPane p = new BorderPane();
p.setCenter(comboBox);

Scene scene = new Scene(p);

Stage stage = new Stage();
stage.setScene(scene);
stage.setWidth(500);
stage.setHeight(400);
stage.show();

Toolkit.getToolkit().firePulse();

for (int i = 0; i < LIST_SIZE; i++) {
comboBox.getItems().add(String.valueOf(i));
}
comboBox.show();

VirtualFlow<IndexedCell<?>> virtualFlow = (VirtualFlow<IndexedCell<?>>) VirtualFlowTestUtils.getVirtualFlow(comboBox);

int index = 0;
comboBox.getSelectionModel().select(index);
Toolkit.getToolkit().firePulse();

int first = virtualFlow.getFirstVisibleCell().getIndex();
int last = virtualFlow.getLastVisibleCell().getIndex();
assertTrue(" visible range [" + first + ", " + last + "] must include " + index,
first <= index && index <= last);

This comment has been minimized.

@aghaisas

aghaisas Apr 17, 2020
Collaborator

You have tested first and last index selection.
It will be nice to have a case where we select index = LIST_SIZE/2

This comment has been minimized.

@ccavanaugh

ccavanaugh Apr 18, 2020
Author

Agreed, mid point test added

index = LIST_SIZE / 2;
comboBox.getSelectionModel().select(index);
Toolkit.getToolkit().firePulse();

first = virtualFlow.getFirstVisibleCell().getIndex();
last = virtualFlow.getLastVisibleCell().getIndex();
assertTrue(" visible range [" + first + ", " + last + "] must include " + index,
first <= index && index <= last);

index = LIST_SIZE - 1;
comboBox.getSelectionModel().select(index);
Toolkit.getToolkit().firePulse();

first = virtualFlow.getFirstVisibleCell().getIndex();
last = virtualFlow.getLastVisibleCell().getIndex();
assertTrue(" visible range [" + first + ", " + last + "] must include " + index,
first <= index && index <= last);
}
}
ProTip! Use n and p to navigate between commits in a pull request.