Don't apply type button to non button elements (#1772)

Author: snowystinger

packages/@react-aria/button/src/useButton.ts

@@ -64,7 +64,12 @@ export function useButton(props: AriaButtonProps<ElementType>, ref: RefObject<an
     type = 'button'
   } = props;
   let additionalProps;
-  if (elementType !== 'button') {
+  if (elementType === 'button') {
+    additionalProps = {
+      type,
+      disabled: isDisabled
+    };
+  } else {
     additionalProps = {
       role: 'button',
       tabIndex: isDisabled ? undefined : 0,
@@ -98,8 +103,6 @@ export function useButton(props: AriaButtonProps<ElementType>, ref: RefObject<an
       'aria-expanded': props['aria-expanded'],
       'aria-controls': props['aria-controls'],
       'aria-pressed': props['aria-pressed'],
-      disabled: isDisabled,
-      type,
       onClick: (e) => {
         if (deprecatedOnClick) {
           deprecatedOnClick(e);

packages/@react-aria/button/test/useButton.test.js

@@ -30,6 +30,16 @@ describe('useButton tests', function () {
     expect(typeof result.current.buttonProps.onKeyDown).toBe('function');
     expect(result.current.buttonProps.rel).toBeUndefined();
   });
+  it('handles elements other than button disabled', function () {
+    let props = {elementType: 'a', isDisabled: true};
+    let {result} = renderHook(() => useButton(props));
+    expect(result.current.buttonProps.role).toBe('button');
+    expect(result.current.buttonProps.tabIndex).toBeUndefined();
+    expect(result.current.buttonProps['aria-disabled']).toBeTruthy();
+    expect(result.current.buttonProps.href).toBeUndefined();
+    expect(typeof result.current.buttonProps.onKeyDown).toBe('function');
+    expect(result.current.buttonProps.rel).toBeUndefined();
+  });
   it('handles the rel attribute on anchors', function () {
     let props = {elementType: 'a', rel: 'noopener noreferrer'};
     let {result} = renderHook(() => useButton(props));
@@ -40,4 +50,15 @@ describe('useButton tests', function () {
     let {result} = renderHook(() => useButton(props));
     expect(result.current.buttonProps.rel).toBe('search');
   });
+  it('handles input elements', function () {
+    let props = {elementType: 'input', isDisabled: true};
+    let {result} = renderHook(() => useButton(props));
+    expect(result.current.buttonProps.role).toBe('button');
+    expect(result.current.buttonProps.tabIndex).toBeUndefined();
+    expect(result.current.buttonProps['aria-disabled']).toBeUndefined();
+    expect(result.current.buttonProps.disabled).toBeTruthy();
+    expect(result.current.buttonProps.href).toBeUndefined();
+    expect(typeof result.current.buttonProps.onKeyDown).toBe('function');
+    expect(result.current.buttonProps.rel).toBeUndefined();
+  });
 });

packages/@react-spectrum/button/test/Button.test.js

@@ -156,6 +156,7 @@ describe('Button', function () {
 
     let button = getByRole('button');
     expect(button).toHaveAttribute('tabindex', '0');
+    expect(button).not.toHaveAttribute('type', 'button');
     triggerPress(button);
     expect(onPressSpy).toHaveBeenCalledTimes(1);
 
@@ -196,6 +197,7 @@ describe('Button', function () {
 
     let button = getByRole('button');
     triggerPress(button);
+    expect(button).toBeDisabled();
     expect(onPressSpy).not.toHaveBeenCalled();
   });
 

packages/@react-spectrum/numberfield/test/NumberField.test.js

@@ -87,6 +87,8 @@ describe('NumberField', function () {
     expect(textField).toHaveAttribute('inputMode', 'numeric');
     expect(incrementButton).toBeTruthy();
     expect(decrementButton).toBeTruthy();
+    expect(incrementButton).toHaveAttribute('tabIndex', '-1');
+    expect(decrementButton).toHaveAttribute('tabIndex', '-1');
   });
 
   it('attaches a user provided ref to the outer div', function () {
@@ -1442,6 +1444,10 @@ describe('NumberField', function () {
     expect(textField).toHaveAttribute('value', '100');
     expect(incrementButton).toHaveAttribute('aria-disabled');
     expect(decrementButton).toHaveAttribute('aria-disabled');
+    // they are aria-disabled, but don't have the attribute disabled because they are not buttons or inputs
+    // should they be made buttons again after the Safari bug is fixed, this will need to be reversed
+    expect(incrementButton).not.toBeDisabled();
+    expect(decrementButton).not.toBeDisabled();
   });
 
   it.each`