feat(FiltersInline): PDD-289 apply filter design feedback

.codex/hooks.json

@@ -0,0 +1,14 @@
+{
+  "hooks": {
+    "SessionStart": [
+      {
+        "hooks": [
+          {
+            "type": "command",
+            "command": "cat ../.agents/skills/seldon/SKILL.md ../.agents/skills/vercel-composition-patterns/SKILL.md"
+          }
+        ]
+      }
+    ]
+  }
+}

src/patterns/FiltersInline/FilterDropdownMenuDesktop.tsx

@@ -1,7 +1,7 @@
 import classnames from 'classnames';
 import React from 'react';
 import Button from '../../components/Button/Button';
-import { ButtonSizes, ButtonVariants } from '../../components/Button/types';
+import { ButtonVariants } from '../../components/Button/types';
 import FilterInput from '../../components/Filter/FilterInput';
 import { px } from '../../utils';
 import { FilterDimension, FilterDropdownMenuProps } from './types';
@@ -45,38 +45,27 @@ export const FilterDropdownMenuDesktop = React.forwardRef<HTMLDivElement, Filter
               type={isSortButton ? 'radio' : 'checkbox'}
               checked={value.active}
               disabled={value?.disabled}
-              onChange={(e) => handleInputChangeUtil(e, buttonType ?? '', onSelectFilter)}
+              onChange={(e) => handleInputChangeUtil(e, buttonType ?? '', onSelectFilter, 'dropdown')}
             />
           ))}
         </div>
-        <div className={classnames(isSortButton ? `${baseClassName}__button-wrap` : `${baseClassName}__buttons-wrap`)}>
-          {isSortButton ? (
+        {/* Sort applies on selection, so the sort dropdown renders no action buttons. */}
+        {!isSortButton && (
+          <div className={classnames(`${baseClassName}__buttons-wrap`)}>
             <Button
-              className={classnames(`${baseClassName}__button`)}
-              variant={ButtonVariants.primary}
-              size={ButtonSizes.small}
-              onClick={() => onApplyFilter?.(false)}
+              variant={ButtonVariants.secondary}
+              isDisabled={!hasActiveFilters}
+              onClick={() => onClickClear?.(buttonType ?? '')}
             >
-              <span className={`${baseClassName}__button-text`}>{dropdownMenuTranslation?.confirm || 'Confirm'}</span>
+              {dropdownMenuTranslation?.clearAll || 'Clear all'}
             </Button>
-          ) : (
-            <>
-              <Button
-                variant={ButtonVariants.secondary}
-                size={ButtonSizes.small}
-                isDisabled={!hasActiveFilters}
-                onClick={() => onClickClear?.(buttonType ?? '')}
-              >
-                {dropdownMenuTranslation?.clearAll || 'Clear all'}
-              </Button>
-              <Button variant={ButtonVariants.primary} size={ButtonSizes.small} onClick={() => onApplyFilter?.(false)}>
-                <span className={`${baseClassName}__button-text`}>
-                  {dropdownMenuTranslation?.showAuctions || `Show ${resultsCount} Auctions`}
-                </span>
-              </Button>
-            </>
-          )}
-        </div>
+            <Button variant={ButtonVariants.primary} onClick={() => onApplyFilter?.(false)}>
+              <span className={`${baseClassName}__button-text`}>
+                {dropdownMenuTranslation?.showAuctions || `Show ${resultsCount} Auctions`}
+              </span>
+            </Button>
+          </div>
+        )}
       </div>
     );
   },

src/patterns/FiltersInline/FilterDropdownMenuMobile.tsx

@@ -1,10 +1,8 @@
 import classnames from 'classnames';
 import React from 'react';
 import Button from '../../components/Button/Button';
-import { ButtonSizes, ButtonVariants } from '../../components/Button/types';
+import { ButtonVariants } from '../../components/Button/types';
 import FilterInput from '../../components/Filter/FilterInput';
-import Text from '../../components/Text/Text';
-import { TextVariants } from '../../components/Text/types';
 import { px } from '../../utils';
 import { FilterDimension, FilterDropdownMenuProps } from './types';
 import { getFilterDimensions, handleInputChange as handleInputChangeUtil, hasActiveDimensions } from './utils';
@@ -47,45 +45,31 @@ export const FilterDropdownMenuMobile = React.forwardRef<HTMLDivElement, FilterD
               type={isSortButton ? 'radio' : 'checkbox'}
               checked={value.active}
               disabled={value?.disabled}
-              onChange={(e) => handleInputChangeUtil(e, buttonType ?? '', onSelectFilter)}
+              onChange={(e) => handleInputChangeUtil(e, buttonType ?? '', onSelectFilter, 'dropdown')}
             />
           ))}
         </div>
-        <div
-          className={classnames(
-            isSortButton ? `${baseClassName}__button-wrap` : `${baseClassName}__buttons-wrap`,
-            `${baseClassName}__mobile-wrap`,
-          )}
-        >
-          {isSortButton ? (
+        {/* Sort applies on selection, so the sort dropdown renders no action
+            buttons — just a spacer so the safe-area inset still keeps the last
+            option clear of the iOS home indicator. */}
+        {isSortButton ? (
+          <div className={`${baseClassName}__mobile-safe-area`} aria-hidden="true" />
+        ) : (
+          <div className={classnames(`${baseClassName}__buttons-wrap`, `${baseClassName}__mobile-wrap`)}>
             <Button
-              className={classnames(`${baseClassName}__button`, `${baseClassName}__button--mobile`)}
-              variant={ButtonVariants.primary}
-              size={ButtonSizes.small}
-              onClick={() => onApplyFilter?.(false)}
+              variant={ButtonVariants.secondary}
+              isDisabled={!hasActiveFilters}
+              onClick={() => onClickClear?.(buttonType ?? '')}
             >
-              <Text variant={TextVariants.headingMedium} className={`${baseClassName}__button-text`}>
-                {dropdownMenuTranslation?.confirm || 'Confirm'}
-              </Text>
+              {dropdownMenuTranslation?.clearAll || 'Clear all'}
             </Button>
-          ) : (
-            <>
-              <Button
-                variant={ButtonVariants.secondary}
-                size={ButtonSizes.small}
-                isDisabled={!hasActiveFilters}
-                onClick={() => onClickClear?.(buttonType ?? '')}
-              >
-                <Text variant={TextVariants.bodySmall}>{dropdownMenuTranslation?.clearAll || 'Clear all'}</Text>
-              </Button>
-              <Button variant={ButtonVariants.primary} size={ButtonSizes.small} onClick={() => onApplyFilter?.(false)}>
-                <Text variant={TextVariants.bodySmall} className={`${baseClassName}__button-text`}>
-                  {dropdownMenuTranslation?.showAuctions || `Show ${resultsCount} Auctions`}
-                </Text>
-              </Button>
-            </>
-          )}
-        </div>
+            <Button variant={ButtonVariants.primary} onClick={() => onApplyFilter?.(false)}>
+              <span className={`${baseClassName}__button-text`}>
+                {dropdownMenuTranslation?.showAuctions || `Show ${resultsCount} Auctions`}
+              </span>
+            </Button>
+          </div>
+        )}
       </div>
     );
   },

src/patterns/FiltersInline/FiltersInline.test.tsx

@@ -135,15 +135,27 @@ describe('FilterDropdown', () => {
     clearFilterUpdate.mockClear();
   });
 
-  it('renders mobile variant', () => {
+  it('renders mobile variant without action buttons for sort', () => {
     render(
       <FilterDropdownMenuMobile
         {...getProps({ isMobileDropdown: true, filterButtonLabel: 'Sort' })}
         filterButtonLabel="Sort"
       />,
     );
     expect(screen.getByTestId('filter-dropdown-mobile')).toBeInTheDocument();
-    expect(screen.getByText('Confirm')).toBeInTheDocument();
+    expect(screen.getByLabelText('Ascending')).toBeInTheDocument();
+    expect(screen.queryByText('Confirm')).not.toBeInTheDocument();
+    expect(screen.queryByRole('button')).not.toBeInTheDocument();
+  });
+
+  it('renders desktop sort variant without action buttons and applies selection via onSelectFilter', () => {
+    const onSelectFilter = vi.fn();
+    render(<FilterDropdownMenuDesktop {...getProps({ onSelectFilter })} filterButtonLabel="Sort" />);
+    expect(screen.queryByText('Confirm')).not.toBeInTheDocument();
+    expect(screen.queryByRole('button')).not.toBeInTheDocument();
+
+    fireEvent.click(screen.getByLabelText('Ascending'));
+    expect(onSelectFilter).toHaveBeenCalledWith(expect.anything(), 'Sort', 'dropdown');
   });
 });
 
@@ -260,6 +272,27 @@ describe('MainFilterDropdown', () => {
     fireEvent.click(screen.getByTestId('main-filter-filter-button'));
     expect(handleClick).toHaveBeenCalled();
   });
+
+  it('marks drawer input changes with the drawer source', () => {
+    render(
+      <MainFilterDropdown
+        id="main-filter"
+        filterButtonLabel="Filter"
+        filterId={0}
+        buttonType={'Filter' as FilterButtonType}
+        handleClick={handleClick}
+        filtersListState={[true]}
+        filters={filters}
+        onSelectFilter={handleFilterSelection}
+        onApplyFilter={handleFilterUpdate}
+        onClickClear={clearFilterUpdate}
+        resultsCount={2}
+        ariaLabels={{ button: 'Filter Button' }}
+      />,
+    );
+    fireEvent.click(screen.getByLabelText('Foo'));
+    expect(handleFilterSelection).toHaveBeenCalledWith(expect.anything(), FilterButtonType.Filter, 'drawer');
+  });
 });
 
 describe('SubFilterDropdown', () => {
@@ -523,7 +556,16 @@ describe('handleInputChange', () => {
 
     handleInputChange(mockEvent, 'Filter', mockHandler);
 
-    expect(mockHandler).toHaveBeenCalledWith(mockEvent, 'Filter');
+    expect(mockHandler).toHaveBeenCalledWith(mockEvent, 'Filter', undefined);
+  });
+
+  it('passes the change source through to handleFilterSelection', () => {
+    const mockEvent = { target: { value: 'foo' } } as React.ChangeEvent<HTMLInputElement>;
+    const mockHandler = vi.fn();
+
+    handleInputChange(mockEvent, 'Sort', mockHandler, 'dropdown');
+
+    expect(mockHandler).toHaveBeenCalledWith(mockEvent, 'Sort', 'dropdown');
   });
 
   it('does nothing if handleFilterSelection is not provided', () => {

src/patterns/FiltersInline/FiltersInline.tsx

@@ -69,25 +69,36 @@ const FiltersInline = React.forwardRef<HTMLDivElement, FiltersInlineProps>(
           resultsCount={resultsCount}
           dropdownMenuTranslation={dropdownMenuTranslation}
         />
-        {filters?.map((filter, index) => (
-          <SubFilterDropdown
-            key={`${id}-${filter.label}-button`}
-            id={`${id}-${filter.label}-button`}
-            filterButtonLabel={filter.label}
-            buttonType={filter.buttonType as unknown as FilterButtonType}
-            handleClick={setFiltersLabelListState}
-            filtersListState={filtersListState}
-            filterId={index + 1}
-            filters={filters}
-            onSelectFilter={onSelectFilter}
-            onApplyFilter={onApplyFilter}
-            onClickClear={onClickClear}
-            resultsCount={resultsCount}
-            filterButtonLabelTranslated={filter.filterButtonLabelTranslated}
-            dropdownMenuTranslation={dropdownMenuTranslation}
-            hideDesktopSortButton={hideDesktopSortButton}
-          />
-        ))}
+        {filters
+          ?.map((filter, index) => ({ filter, filterId: index + 1 }))
+          // Render the Sort pill last in the DOM so keyboard/reading order
+          // matches the visual order. Sort is listed first for the drawer, but
+          // sits last (right-aligned) in the row; a stable sort keeps the other
+          // pills in their original order.
+          .sort(
+            (a, b) =>
+              (a.filter.buttonType === FilterButtonType.Sort ? 1 : 0) -
+              (b.filter.buttonType === FilterButtonType.Sort ? 1 : 0),
+          )
+          .map(({ filter, filterId }) => (
+            <SubFilterDropdown
+              key={`${id}-${filter.label}-button`}
+              id={`${id}-${filter.label}-button`}
+              filterButtonLabel={filter.label}
+              buttonType={filter.buttonType as unknown as FilterButtonType}
+              handleClick={setFiltersLabelListState}
+              filtersListState={filtersListState}
+              filterId={filterId}
+              filters={filters}
+              onSelectFilter={onSelectFilter}
+              onApplyFilter={onApplyFilter}
+              onClickClear={onClickClear}
+              resultsCount={resultsCount}
+              filterButtonLabelTranslated={filter.filterButtonLabelTranslated}
+              dropdownMenuTranslation={dropdownMenuTranslation}
+              hideDesktopSortButton={hideDesktopSortButton}
+            />
+          ))}
       </div>
     );
   },

src/patterns/FiltersInline/MainFilterDropdown.tsx

@@ -1,7 +1,7 @@
 import classnames from 'classnames';
 import React from 'react';
 import Button from '../../components/Button/Button';
-import { ButtonSizes, ButtonVariants } from '../../components/Button/types';
+import { ButtonVariants } from '../../components/Button/types';
 import Drawer from '../../components/Drawer/Drawer';
 import Filter from '../../components/Filter/Filter';
 import FilterHeader from '../../components/Filter/FilterHeader';
@@ -64,7 +64,7 @@ export const MainFilterDropdown = React.forwardRef<HTMLButtonElement, FilterDrop
                     id={value.label}
                     key={value.label}
                     labelText={value.label}
-                    onChange={(e) => onSelectFilter?.(e, filter.buttonType)}
+                    onChange={(e) => onSelectFilter?.(e, filter.buttonType, 'drawer')}
                     type={filter.type as 'checkbox' | 'radio'}
                     disabled={value?.disabled}
                     name={value.label}
@@ -82,13 +82,12 @@ export const MainFilterDropdown = React.forwardRef<HTMLButtonElement, FilterDrop
           >
             <Button
               variant={ButtonVariants.secondary}
-              size={ButtonSizes.small}
               isDisabled={totalCount === 0}
               onClick={() => onClickClear?.('all')}
             >
               {dropdownMenuTranslation?.clearAll || 'Clear all'}
             </Button>
-            <Button size={ButtonSizes.small} onClick={() => onApplyFilter?.(false)}>
+            <Button onClick={() => onApplyFilter?.(false)}>
               {dropdownMenuTranslation?.showAuctions || `Show ${resultsCount} Auctions`}
             </Button>
           </div>