Fix borderRadius-related bug in [ButtonGroup] (#62)

Add library givens Fix borderRadius-related bugs * I fould borderRadius is not applied properly when data size is 1. So I can't trust code that sets border width and radius. Write test code about border, and rewrite original code. * There's some trivial changes too. I mentioned it in Pull Request. * Remove unrelated changes
hyochan · Aug 10, 2021 · f41047a · f41047a
1 parent 63175fd
commit f41047a
Show file tree

Hide file tree

Showing 6 changed files with 181 additions and 31 deletions.
diff --git a/main/ButtonGroup.tsx b/main/ButtonGroup.tsx
@@ -45,6 +45,57 @@ function StyledButtonGroup<T>(props: Props<T>): React.ReactElement {
     styles,
   } = props;
 
+  const borderWidthAndRadius = (index: number): object => {
+    const fullWidthAndRadius = {
+      borderLeftWidth: borderWidth,
+      borderRightWidth: borderWidth,
+      borderTopWidth: borderWidth,
+      borderBottomWidth: borderWidth,
+
+      borderTopLeftRadius: borderRadius,
+      borderBottomLeftRadius: borderRadius,
+      borderTopRightRadius: borderRadius,
+      borderBottomRightRadius: borderRadius,
+    };
+
+    const isFirst = index === 0;
+    const isLast = index === data.length - 1;
+
+    const borderForFirstElement = {
+      ...fullWidthAndRadius,
+      borderTopRightRadius: undefined,
+      borderBottomRightRadius: undefined,
+    };
+
+    const borderForLastElement = {
+      ...fullWidthAndRadius,
+      borderTopLeftRadius: undefined,
+      borderBottomLeftRadius: undefined,
+    };
+
+    const borderForMiddleElement = {
+      borderRightWidth: borderWidth,
+      borderTopWidth: borderWidth,
+      borderBottomWidth: borderWidth,
+    };
+
+    if (data.length === 1) return fullWidthAndRadius;
+
+    if (isFirst) return borderForFirstElement;
+
+    if (isLast) {
+      if (data.length === 2)
+        return {
+          ...borderForLastElement,
+          borderLeftWidth: undefined,
+        };
+
+      return borderForLastElement;
+    }
+
+    return borderForMiddleElement;
+  };
+
   return (
     <View
       testID={testID}
@@ -57,39 +108,18 @@ function StyledButtonGroup<T>(props: Props<T>): React.ReactElement {
         return (
           <TouchableOpacity
             key={i}
-            testID={`CHILD_${i}`}
             activeOpacity={0.85}
             style={{flex: 1}}
             onPress={(): void => {
               if (onPress) onPress(i);
             }}>
             <View
+              testID={`CHILD_${i}`}
               style={StyleSheet.flatten([
                 selectedIndex === i
                   ? {...styles?.selectedButton, backgroundColor: color}
                   : {...styles?.button, borderColor: color},
-                i === 0
-                  ? {
-                      borderLeftWidth: borderWidth,
-                      borderTopWidth: borderWidth,
-                      borderBottomWidth: borderWidth,
-                      borderTopLeftRadius: borderRadius,
-                      borderBottomLeftRadius: borderRadius,
-                    }
-                  : i === data.length - 1
-                  ? {
-                      borderRightWidth: borderWidth,
-                      borderLeftWidth: borderWidth,
-                      borderTopWidth: borderWidth,
-                      borderBottomWidth: borderWidth,
-                      borderBottomRightRadius: borderRadius,
-                      borderTopRightRadius: borderRadius,
-                    }
-                  : {
-                      borderLeftWidth: borderWidth,
-                      borderTopWidth: borderWidth,
-                      borderBottomWidth: borderWidth,
-                    },
+                borderWidthAndRadius(i),
                 {borderColor: color},
               ])}>
               <Text

diff --git a/main/__tests__/ButtonGroup.test.tsx b/main/__tests__/ButtonGroup.test.tsx
@@ -1,3 +1,4 @@
+/* eslint-disable jest/no-identical-title */
 import {fireEvent, render, RenderAPI} from '@testing-library/react-native';
 import {createComponent, createTestProps} from '../../test/testUtils';
 
@@ -10,13 +11,17 @@ describe('[ButtonGroup]', () => {
 
   const handlePress = jest.fn();
 
-  const renderButtonGroup = (i?: number): RenderAPI =>
+  const renderButtonGroup = (): RenderAPI =>
     render(
       <ThemeProvider initialThemeType={themeType}>
         <ButtonGroup
-          selectedIndex={i}
-          data={['option 1', 'option 2']}
+          data={given.data ?? ['option 1', 'option 2']}
+          selectedIndex={given.selectedIndex}
           onPress={handlePress}
+          borderWidth={given.borderWidth}
+          borderRadius={given.borderRadius}
+          style={given.style}
+          styles={given.styles}
         />
       </ThemeProvider>,
     );
@@ -56,10 +61,10 @@ describe('[ButtonGroup]', () => {
   });
 
   context('when selectedIndex is provided', () => {
-    const selectedIndex = 1;
+    given('selectedIndex', () => 1);
 
     it('makes selected option text notable', () => {
-      const {getByText} = renderButtonGroup(selectedIndex);
+      const {getByText} = renderButtonGroup();
 
       expect(getByText('option 1')).toHaveStyle({
         color: theme[themeType].text,
@@ -70,4 +75,106 @@ describe('[ButtonGroup]', () => {
       });
     });
   });
+
+  describe('Border', () => {
+    given('borderWidth', () => 1);
+    given('borderRadius', () => 10);
+
+    const fullWidthAndRadius = {
+      borderLeftWidth: 1,
+      borderRightWidth: 1,
+      borderTopWidth: 1,
+      borderBottomWidth: 1,
+
+      borderTopLeftRadius: 10,
+      borderTopRightRadius: 10,
+      borderBottomLeftRadius: 10,
+      borderBottomRightRadius: 10,
+    };
+
+    context('when data size is 1', () => {
+      given('data', () => ['Option 1']);
+
+      it('has width and radius in every direction', () => {
+        const {getByTestId} = renderButtonGroup();
+
+        expect(getByTestId('CHILD_0')).toHaveStyle(fullWidthAndRadius);
+      });
+    });
+
+    context('when data size is 2', () => {
+      given('data', () => ['Option 1', 'Option 2']);
+
+      it('depends on position of element', () => {
+        const {getByTestId} = renderButtonGroup();
+
+        expect(getByTestId('CHILD_0')).toHaveStyle({
+          ...fullWidthAndRadius,
+          borderTopRightRadius: undefined,
+          borderBottomRightRadius: undefined,
+        });
+
+        expect(getByTestId('CHILD_0')).not.toHaveStyle({
+          borderTopRightRadius: 10,
+          borderBottomRightRadius: 10,
+        });
+
+        expect(getByTestId('CHILD_1')).toHaveStyle({
+          ...fullWidthAndRadius,
+          borderLeftWidth: undefined,
+          borderTopLeftRadius: undefined,
+          borderBottomLeftRadius: undefined,
+        });
+
+        expect(getByTestId('CHILD_1')).not.toHaveStyle({
+          borderLeftWidth: 1,
+          borderTopLeftRadius: 10,
+          borderBottomLeftRadius: 10,
+        });
+      });
+    });
+
+    context('when data size is 3', () => {
+      given('data', () => ['Option 1', 'Option 2', 'Option 3']);
+
+      it('depends on position of element', () => {
+        const {getByTestId} = renderButtonGroup();
+
+        expect(getByTestId('CHILD_0')).toHaveStyle({
+          ...fullWidthAndRadius,
+          borderTopRightRadius: undefined,
+          borderBottomRightRadius: undefined,
+        });
+
+        expect(getByTestId('CHILD_0')).not.toHaveStyle({
+          borderTopRightRadius: 10,
+          borderBottomRightRadius: 10,
+        });
+
+        expect(getByTestId('CHILD_1')).toHaveStyle({
+          borderRightWidth: 1,
+          borderTopWidth: 1,
+          borderBottomWidth: 1,
+        });
+
+        expect(getByTestId('CHILD_1')).not.toHaveStyle({
+          ...fullWidthAndRadius,
+          borderRightWidth: undefined,
+          borderTopWidth: undefined,
+          borderBottomWidth: undefined,
+        });
+
+        expect(getByTestId('CHILD_2')).toHaveStyle({
+          ...fullWidthAndRadius,
+          borderTopLeftRadius: undefined,
+          borderBottomLeftRadius: undefined,
+        });
+
+        expect(getByTestId('CHILD_2')).not.toHaveStyle({
+          borderTopLeftRadius: 10,
+          borderBottomLeftRadius: 10,
+        });
+      });
+    });
+  });
 });
diff --git a/main/__tests__/__snapshots__/ButtonGroup.test.tsx.snap b/main/__tests__/__snapshots__/ButtonGroup.test.tsx.snap
@@ -31,7 +31,6 @@ exports[`[ButtonGroup] renders without crashing 1`] = `
         "opacity": 1,
       }
     }
-    testID="CHILD_0"
   >
     <View
       style={
@@ -40,15 +39,19 @@ exports[`[ButtonGroup] renders without crashing 1`] = `
           "alignSelf": "stretch",
           "backgroundColor": "black",
           "borderBottomLeftRadius": 0,
+          "borderBottomRightRadius": undefined,
           "borderBottomWidth": 1,
           "borderColor": "black",
           "borderLeftWidth": 1,
+          "borderRightWidth": 1,
           "borderTopLeftRadius": 0,
+          "borderTopRightRadius": undefined,
           "borderTopWidth": 1,
           "justifyContent": "center",
           "minHeight": 40,
         }
       }
+      testID="CHILD_0"
     >
       <Text
         style={
@@ -84,24 +87,26 @@ exports[`[ButtonGroup] renders without crashing 1`] = `
         "opacity": 1,
       }
     }
-    testID="CHILD_1"
   >
     <View
       style={
         Object {
           "alignItems": "center",
           "alignSelf": "stretch",
+          "borderBottomLeftRadius": undefined,
           "borderBottomRightRadius": 0,
           "borderBottomWidth": 1,
           "borderColor": "black",
-          "borderLeftWidth": 1,
+          "borderLeftWidth": undefined,
           "borderRightWidth": 1,
+          "borderTopLeftRadius": undefined,
           "borderTopRightRadius": 0,
           "borderTopWidth": 1,
           "justifyContent": "center",
           "minHeight": 40,
         }
       }
+      testID="CHILD_1"
     >
       <Text
         style={

diff --git a/package.json b/package.json
@@ -73,6 +73,7 @@
     "expo-font": "~9.2.1",
     "expo-linear-gradient": "~9.2.0",
     "gh-pages": "^3.2.3",
+    "givens": "^1.3.9",
     "intl": "^1.2.5",
     "jest": "^27.0.6",
     "jest-expo": "^42.1.0",

diff --git a/test/setupTest.ts b/test/setupTest.ts
@@ -4,6 +4,8 @@ import '@testing-library/jest-native/extend-expect';
 
 import 'jest-plugin-context/setup';
 
+import 'givens/setup';
+
 jest.mock('react-native/Libraries/Animated/src/NativeAnimatedHelper');
 
 // Cleanup after each case.

diff --git a/yarn.lock b/yarn.lock
@@ -10641,6 +10641,11 @@ gitconfiglocal@^1.0.0:
   dependencies:
     ini "^1.3.2"
 
+givens@^1.3.9:
+  version "1.3.9"
+  resolved "https://registry.yarnpkg.com/givens/-/givens-1.3.9.tgz#2061c02e01b1d0322e6854e6a99919bb91077a0d"
+  integrity sha512-4hYlStsEIaYeYvZTZwgD5yOS2WVP0dcDsOBqeImdEM8eLuclvv0IEMlQQ1kuA5DN4he7wVH1jsYtNe9uininxg==
+
 glob-base@^0.3.0:
   version "0.3.0"
   resolved "https://registry.yarnpkg.com/glob-base/-/glob-base-0.3.0.tgz#dbb164f6221b1c0b1ccf82aea328b497df0ea3c4"