remove addAllOption function from Select

trento-project · Nov 21, 2023 · 40ed6f0 · 40ed6f0
1 parent 8e02b2a
commit 40ed6f0
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 67 deletions.
diff --git a/assets/js/components/ChecksCatalog/ChecksCatalog.jsx b/assets/js/components/ChecksCatalog/ChecksCatalog.jsx
@@ -27,9 +27,8 @@ function ChecksCatalog({ catalogData, catalogError, loading, updateCatalog }) {
         <Select
           optionsName="providers"
           className="ml-auto"
-          options={providers}
-          withAllOption
-          optionRenderer={providerOptionRenderer}
+          options={[OPTION_ALL, ...providers]}
+          renderOption={providerOptionRenderer}
           value={selectedProvider}
           onChange={setProviderSelected}
         />

diff --git a/assets/js/components/Select/Select.jsx b/assets/js/components/Select/Select.jsx
@@ -7,26 +7,23 @@ import classNames from 'classnames';
 
 export const OPTION_ALL = 'all';
 
-const addAllOption = (items) => [OPTION_ALL, ...items];
 const defaultOnChange = () => {};
-const defaultOptionRenderer = (item) => item;
-const renderOption = (item, optionsName, withAllOption, optionRenderer) => {
-  if (withAllOption && item === OPTION_ALL) {
+const defaultRenderOption = (item) => item;
+const doRenderOption = (option, optionsName, renderOption) => {
+  if (option === OPTION_ALL) {
     return `All ${optionsName}`;
   }
-  return optionRenderer(item);
+  return renderOption(option);
 };
 
 function Select({
   optionsName,
   options,
   value,
-  optionRenderer = defaultOptionRenderer,
+  renderOption = defaultRenderOption,
   onChange = defaultOnChange,
-  withAllOption = false,
   className,
 }) {
-  const allAptions = withAllOption ? addAllOption(options) : options;
   const dropdownSelector = `${optionsName.replace(
     /\s+/g,
     ''
@@ -38,7 +35,7 @@ function Select({
           <Listbox.Button
             className={`${dropdownSelector} relative w-full py-2 pl-3 pr-10 text-left bg-white rounded-lg cursor-default border border-gray-300 focus:outline-none focus-visible:ring-2 focus-visible:ring-opacity-75 focus-visible:ring-white focus-visible:ring-offset-orange-300 focus-visible:ring-offset-2 focus-visible:border-indigo-500 sm:text-sm`}
           >
-            {renderOption(value, optionsName, withAllOption, optionRenderer)}
+            {doRenderOption(value, optionsName, renderOption)}
             <span className="absolute inset-y-0 right-0 flex items-center pr-2 pointer-events-none">
               <ChevronUpDownIcon
                 className="w-5 h-5 text-gray-400"
@@ -53,7 +50,7 @@ function Select({
             leaveTo="opacity-0"
           >
             <Listbox.Options className="absolute w-full py-1 mt-1 overflow-auto text-base bg-white rounded-md shadow-lg max-h-60 ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm z-[1]">
-              {allAptions.map((option) => (
+              {options.map((option) => (
                 <Listbox.Option
                   key={option}
                   className={({ active }) =>
@@ -70,12 +67,7 @@ function Select({
                           isSelected ? 'font-medium' : 'font-normal'
                         }`}
                       >
-                        {renderOption(
-                          option,
-                          optionsName,
-                          withAllOption,
-                          optionRenderer
-                        )}
+                        {doRenderOption(option, optionsName, renderOption)}
                       </span>
                       {isSelected ? (
                         <span className="absolute inset-y-0 left-0 flex items-center pl-3 text-green-600">

diff --git a/assets/js/components/Select/Select.stories.jsx b/assets/js/components/Select/Select.stories.jsx
@@ -30,19 +30,12 @@ export default {
         type: 'text',
       },
     },
-    optionRenderer: {
+    renderOption: {
       description: 'A function to render each option in the dropdown',
       table: {
         type: { summary: '(item) => item' },
       },
     },
-    withAllOption: {
-      description:
-        'Whether the dropdown should have an "All `optionsName`" option',
-      control: {
-        type: 'boolean',
-      },
-    },
     onChange: {
       description: 'A function to be called when the selected option changes',
       table: {
@@ -79,7 +72,7 @@ export const Default = {
     optionsName: 'emojis',
     options: emojiOptions,
     value: 'bar',
-    optionRenderer: itemsOptionRenderer,
+    renderOption: itemsOptionRenderer,
   },
 };
 
@@ -90,9 +83,8 @@ const providerOptionRenderer = (provider) => (
 export const WithAllOption = {
   args: {
     optionsName: 'providers',
-    options: providers,
+    options: ['all', ...providers],
     value: 'all',
-    withAllOption: true,
-    optionRenderer: providerOptionRenderer,
+    renderOption: providerOptionRenderer,
   },
 };
diff --git a/assets/js/components/Select/Select.test.jsx b/assets/js/components/Select/Select.test.jsx
@@ -11,45 +11,30 @@ describe('Select Component', () => {
       optionsName: 'foos',
       options: ['foo1', 'foo2', 'foo3'],
       value: 'foo3',
-      withAllOption: false,
       selectedValue: 'foo3',
       allOptions: ['foo1', 'foo2', 'foo3'],
     },
     {
       optionsName: 'bars',
-      options: ['bar1', 'bar2', 'bar3'],
+      options: ['all', 'bar1', 'bar2', 'bar3'],
       value: 'all',
-      withAllOption: true,
       selectedValue: 'All bars',
       allOptions: ['All bars', 'bar1', 'bar2', 'bar3'],
     },
     {
       optionsName: 'foobars',
-      options: ['foobar1', 'foobar2', 'foobar3'],
+      options: ['all', 'foobar1', 'foobar2', 'foobar3'],
       value: 'foobar1',
-      withAllOption: true,
       selectedValue: 'foobar1',
       allOptions: ['All foobars', 'foobar1', 'foobar2', 'foobar3'],
     },
   ];
 
   it.each(scenarios)(
     'should render the selected option',
-    ({
-      optionsName,
-      options,
-      withAllOption,
-      value,
-      selectedValue,
-      allOptions,
-    }) => {
+    ({ optionsName, options, value, selectedValue, allOptions }) => {
       render(
-        <Select
-          optionsName={optionsName}
-          options={options}
-          value={value}
-          withAllOption={withAllOption}
-        />
+        <Select optionsName={optionsName} options={options} value={value} />
       );
       expect(screen.getByRole('button')).toHaveTextContent(selectedValue);
 
@@ -63,22 +48,10 @@ describe('Select Component', () => {
 
   it.each(scenarios)(
     'should render all the options when opened',
-    async ({
-      optionsName,
-      options,
-      withAllOption,
-      value,
-      selectedValue,
-      allOptions,
-    }) => {
+    async ({ optionsName, options, value, selectedValue, allOptions }) => {
       const user = userEvent.setup();
       render(
-        <Select
-          optionsName={optionsName}
-          options={options}
-          value={value}
-          withAllOption={withAllOption}
-        />
+        <Select optionsName={optionsName} options={options} value={value} />
       );
       await user.click(screen.getByText(selectedValue));
 
@@ -102,9 +75,8 @@ describe('Select Component', () => {
       <Select
         optionsName="foobars"
         options={options}
-        withAllOption
         value="all"
-        optionRenderer={optionRenderer}
+        renderOption={optionRenderer}
       />
     );
 
@@ -125,7 +97,6 @@ describe('Select Component', () => {
       <Select
         optionsName="foobars"
         options={options}
-        withAllOption
         value="all"
         onChange={mockOnChange}
       />