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 <[email protected]>
vaadin · Sep 23, 2024 · 34af0c7 · 34af0c7
1 parent bf23101
commit 34af0c7
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -652,6 +652,12 @@
                 <version>4.8.0</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
+                <groupId>org.mockito</groupId>
+                <artifactId>mockito-inline</artifactId>
+                <version>4.8.0</version>
+                <scope>test</scope>
+            </dependency>
             <dependency>
                 <groupId>org.hamcrest</groupId>
                 <artifactId>hamcrest-all</artifactId>

diff --git a/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml b/vaadin-tabs-flow-parent/vaadin-tabs-flow/pom.xml
@@ -47,6 +47,16 @@
             <artifactId>slf4j-simple</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-inline</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     <build>
         <plugins>

diff --git a/...-tabs-flow-parent/vaadin-tabs-flow/src/main/java/com/vaadin/flow/component/tabs/Tabs.java b/...-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/...-parent/vaadin-tabs-flow/src/test/java/com/vaadin/flow/component/tabs/tests/TabsTest.java b/...-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<LoggerFactory> 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);