From 34af0c75ba9f6f8f477dad24394dbc088d7c6182 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Mon, 23 Sep 2024 09:40:35 +0200 Subject: [PATCH] fix: warn and revert to prev index when selecting invalid index (#6543) (CP: 23.5) (#6664) * fix: warn and revert to prev index when selecting invalid index (#6543) * replace getTabCount() with old getComponentCount() * add mockito dependency * add mockito-inline dependecy --------- Co-authored-by: Sergey Vinogradov --- pom.xml | 6 +++ .../vaadin-tabs-flow/pom.xml | 10 +++++ .../com/vaadin/flow/component/tabs/Tabs.java | 17 +++++++- .../flow/component/tabs/tests/TabsTest.java | 39 +++++++++++++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index efe4e5c52c3..23f22731be7 100644 --- a/pom.xml +++ b/pom.xml @@ -652,6 +652,12 @@ 4.8.0 test + + org.mockito + mockito-inline + 4.8.0 + test + org.hamcrest hamcrest-all diff --git a/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml b/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml index f515acd2669..1b7f3d6e244 100644 --- a/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml +++ b/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml @@ -47,6 +47,16 @@ slf4j-simple test + + org.mockito + mockito-core + test + + + org.mockito + mockito-inline + test + diff --git a/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/main/java/com/vaadin/flow/component/tabs/Tabs.java b/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/main/java/com/vaadin/flow/component/tabs/Tabs.java index a41a57c67a1..114256d4403 100644 --- a/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/main/java/com/vaadin/flow/component/tabs/Tabs.java +++ b/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/main/java/com/vaadin/flow/component/tabs/Tabs.java @@ -20,6 +20,8 @@ import java.util.Objects; import java.util.stream.Stream; +import org.slf4j.LoggerFactory; + import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.ClientCallable; import com.vaadin.flow.component.Component; @@ -76,8 +78,19 @@ public enum Orientation { */ public Tabs() { setSelectedIndex(-1); - getElement().addPropertyChangeListener(SELECTED, - event -> updateSelectedTab(event.isUserOriginated())); + getElement().addPropertyChangeListener(SELECTED, event -> { + int oldIndex = selectedTab != null ? indexOf(selectedTab) : -1; + int newIndex = getSelectedIndex(); + if (newIndex >= getComponentCount()) { + LoggerFactory.getLogger(getClass()).warn(String.format( + "The selected index is out of range: %d. Reverting to the previous index: %d.", + newIndex, oldIndex)); + setSelectedIndex(oldIndex); + return; + } + + updateSelectedTab(event.isUserOriginated()); + }); } /** diff --git a/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/test/java/com/vaadin/flow/component/tabs/tests/TabsTest.java b/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/test/java/com/vaadin/flow/component/tabs/tests/TabsTest.java index de95c3b10d2..df995301775 100644 --- a/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/test/java/com/vaadin/flow/component/tabs/tests/TabsTest.java +++ b/vaadin-tabs-flow-parent/vaadin-tabs-flow/src/test/java/com/vaadin/flow/component/tabs/tests/TabsTest.java @@ -23,6 +23,10 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.vaadin.flow.component.html.Div; import com.vaadin.flow.component.tabs.Tab; @@ -103,6 +107,41 @@ public void selectTabByIndex() { assertThat("Selected index is invalid", tabs.getSelectedIndex(), is(2)); } + @Test + public void selectInvalidIndex_previousIndexIsReverted() { + Tab tab = new Tab("Tab"); + Tabs tabs = new Tabs(tab); + + // Select index out of range + tabs.setSelectedIndex(10); + Assert.assertEquals(0, tabs.getSelectedIndex()); + + // Deselect the active tab + tabs.setSelectedIndex(-1); + // Select index out of range + tabs.setSelectedIndex(10); + Assert.assertEquals(-1, tabs.getSelectedIndex()); + } + + @Test + public void selectInvalidIndex_warningIsShown() { + Tab tab = new Tab("Tab"); + Tabs tabs = new Tabs(tab); + + Logger mockedLogger = Mockito.mock(Logger.class); + try (MockedStatic context = Mockito + .mockStatic(LoggerFactory.class)) { + context.when(() -> LoggerFactory.getLogger(Tabs.class)) + .thenReturn(mockedLogger); + + // Select index out of range + tabs.setSelectedIndex(10); + + Mockito.verify(mockedLogger, Mockito.times(1)).warn( + "The selected index is out of range: 10. Reverting to the previous index: 0."); + } + } + @Test public void shouldThrowWhenTabToSelectIsNotChild() { thrown.expect(IllegalArgumentException.class);