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

hand-picked fix to #5043 combobox suggestion popup on scroll #10307

Conversation

OlliTietavainenVaadin
Copy link
Member

@OlliTietavainenVaadin OlliTietavainenVaadin commented Nov 9, 2017

Slightly modified version of #9869 for Vaadin 7.7


This change is Reviewable

@wbadam
Copy link
Contributor

wbadam commented Nov 10, 2017

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 345 at r1 (raw file):

        private final MouseWheeler mouseWheeler = new MouseWheeler();
        

Please remove spaces


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 489 at r1 (raw file):

            return val | 0;
        }-*/;
        

Please remove spaces


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 500 at r1 (raw file):

                    .getLeft());
        }
        

Please remove spaces


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 720 at r1 (raw file):

            super.onPreviewNativeEvent(event);
        }
        

Please remove spaces


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 727 at r1 (raw file):

        private void updatePopupPositionOnScroll() {
            if (!scrollPending) {
                AnimationScheduler.get().requestAnimationFrame(new AnimationCallback() {                    

Please remove spaces


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 740 at r1 (raw file):

            }
        }
        

Please remove spaces


uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java, line 39 at r1 (raw file):

                combobox3, combobox4);
        addComponent(hl);
        

Please remove spaces


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 28 at r1 (raw file):

import com.vaadin.testbench.elements.UIElement;
import com.vaadin.tests.tb3.MultiBrowserTest;
import com.vaadin.tests.tb3.newelements.ComboBoxElement;

Is there a significant difference between this and com.vaadin.testbench.elements.ComboBoxElement?


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 69 at r1 (raw file):

            sleep(500);
        } catch (InterruptedException ignored) {
            

On master, TB throws throw new RuntimeException(e); here. Not sure if it's important.


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 80 at r1 (raw file):

                newPopupLocation.y - popupLocation.y);
    }
    

Please remove spaces


Comments from Reviewable

@OlliTietavainenVaadin
Copy link
Member Author

Review status: 0 of 3 files reviewed at latest revision, 10 unresolved discussions.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 345 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 489 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 500 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 720 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 727 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


client/src/main/java/com/vaadin/client/ui/VFilterSelect.java, line 740 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


uitest/src/main/java/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java, line 39 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 28 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Is there a significant difference between this and com.vaadin.testbench.elements.ComboBoxElement?

No


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 69 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

On master, TB throws throw new RuntimeException(e); here. Not sure if it's important.

Done.


uitest/src/test/java/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java, line 80 at r1 (raw file):

Previously, wbadam (Adam Wagner) wrote…

Please remove spaces

Done.


Comments from Reviewable

@wbadam
Copy link
Contributor

wbadam commented Nov 10, 2017

:lgtm:


Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@OlliTietavainenVaadin OlliTietavainenVaadin merged commit 7bb07cc into vaadin:7.7 Nov 13, 2017
@OlliTietavainenVaadin OlliTietavainenVaadin added this to the 7.7.12 milestone Nov 13, 2017
@OlliTietavainenVaadin OlliTietavainenVaadin deleted the v7-move-combobox-popup-on-scroll branch November 13, 2017 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants