fix: fix review issues

gemini-testing · Aug 23, 2024 · 188a70f · 188a70f
1 parent e16ab11
commit 188a70f
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 24 deletions.
diff --git a/lib/static/new-ui/features/suites/components/BrowsersSelect/index.module.css b/lib/static/new-ui/features/suites/components/BrowsersSelect/index.module.css
@@ -2,9 +2,9 @@
     z-index: 9999;
 }
 
-    .browserlist__filter {
-        padding: 4px;
-    }
+.browserlist__filter {
+    padding: 4px;
+}
 
 .action-button {
     width: 60px;
@@ -15,7 +15,7 @@
     --g-color-text-info: var(--g-color-private-color-500-solid);
 }
 
-    .browserlist__popup :global(.g-select-list__option) {
-        gap: 8px;
-        flex-direction: row-reverse;
-    }
+.browserlist__popup :global(.g-select-list__option) {
+    gap: 8px;
+    flex-direction: row-reverse;
+}
diff --git a/lib/static/new-ui/features/suites/components/BrowsersSelect/index.tsx b/lib/static/new-ui/features/suites/components/BrowsersSelect/index.tsx
@@ -69,16 +69,18 @@ function BrowsersSelectInternal({browsers, filteredBrowsers, actions}: BrowsersS
         const browsersWithMultipleVersions = browsers.filter(browser => browser.versions.length > 1);
         const browsersWithSingleVersion = browsers.filter(browser => browser.versions.length === 1);
 
+        const getOptionProps = (browser: BrowserItem, version: string): {value: string; content: string; data: Record<string, unknown>} => ({
+            value: serializeBrowserData(browser.id, version),
+            content: browser.id,
+            data: {id: browser.id, version}
+        });
+
         if (browsersWithMultipleVersions.length === 0) {
             // If there are no browsers with multiple versions, we want to render a simple plain list
-            return browsers.map(browser => (
-                <Select.Option
-                    key={serializeBrowserData(browser.id, browser.versions[0])}
-                    value={serializeBrowserData(browser.id, browser.versions[0])}
-                    content={browser.id}
-                    data={{id: browser.id, version: browser.versions[0]}}
-                />
-            ));
+            return browsers.map(browser => <Select.Option
+                key={browser.id}
+                {...getOptionProps(browser, browser.versions[0])}
+            />);
         } else {
             // Otherwise render browser version groups and place all browsers with single version into "Other" group
             return (
@@ -87,21 +89,17 @@ function BrowsersSelectInternal({browsers, filteredBrowsers, actions}: BrowsersS
                         <Select.OptionGroup key={browser.id} label={browser.id}>
                             {browser.versions.map(version => (
                                 <Select.Option
-                                    key={serializeBrowserData(browser.id, version)}
-                                    value={serializeBrowserData(browser.id, version)}
-                                    content={`${browser.id} ${version}`}
-                                    data={{id: browser.id, version}}
+                                    key={version}
+                                    {...getOptionProps(browser, version)}
                                 />
                             ))}
                         </Select.OptionGroup>
                     ))}
                     <Select.OptionGroup label="Other">
                         {browsersWithSingleVersion.map(browser => (
                             <Select.Option
-                                key={serializeBrowserData(browser.id, browser.versions[0])}
-                                value={serializeBrowserData(browser.id, browser.versions[0])}
-                                content={browser.id}
-                                data={{id: browser.id, version: browser.versions[0]}}
+                                key={browser.id}
+                                {...getOptionProps(browser, browser.versions[0])}
                             />
                         ))}
                     </Select.OptionGroup>
@@ -119,7 +117,7 @@ function BrowsersSelectInternal({browsers, filteredBrowsers, actions}: BrowsersS
     const selected = selectedBrowsers.flatMap(browser => browser.versions.map(version => serializeBrowserData(browser.id, version)));
 
     const onClose = (): void => {
-        actions.selectBrowsers(selectedBrowsers);
+        actions.selectBrowsers(selectedBrowsers.filter(browser => browser.versions.length > 0));
     };
 
     const onFocus = (): void => {