Merge pull request #773 from Shopify/fix-757

Ensure Buttons with a url prop output valid HTML

UNRELEASED.md

@@ -16,6 +16,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Bug fixes
 
+- Ensure disabled `Button` components with a `url` prop output valid HTML ([#773](https://github.com/Shopify/polaris-react/pull/773))
+
 ### Documentation
 
 - Added deprecation guidelines ([#853](https://github.com/Shopify/polaris-react/pull/853))

src/components/Button/Button.tsx

@@ -152,22 +152,32 @@ function Button({
 
   const type = submit ? 'submit' : 'button';
 
-  return url ? (
-    <UnstyledLink
-      id={id}
-      url={url}
-      external={external}
-      onClick={onClick}
-      onFocus={onFocus}
-      onBlur={onBlur}
-      onMouseUp={handleMouseUpByBlurring}
-      className={className}
-      disabled={isDisabled}
-      aria-label={accessibilityLabel}
-    >
-      {content}
-    </UnstyledLink>
-  ) : (
+  if (url) {
+    return isDisabled ? (
+      // Render an `<a>` so toggling disabled/enabled state changes only the
+      // `href` attribute instead of replacing the whole element.
+      // eslint-disable-next-line jsx-a11y/anchor-is-valid
+      <a id={id} className={className} aria-label={accessibilityLabel}>
+        {content}
+      </a>
+    ) : (
+      <UnstyledLink
+        id={id}
+        url={url}
+        external={external}
+        onClick={onClick}
+        onFocus={onFocus}
+        onBlur={onBlur}
+        onMouseUp={handleMouseUpByBlurring}
+        className={className}
+        aria-label={accessibilityLabel}
+      >
+        {content}
+      </UnstyledLink>
+    );
+  }
+
+  return (
     <button
       id={id}
       type={type}

src/components/Button/tests/Button.test.tsx

@@ -60,30 +60,30 @@ describe('<Button />', () => {
   });
 
   describe('disabled', () => {
-    it('sets the disabled attribute on the button', () => {
+    it('disable without a url renders <button disabled>', () => {
       const button = shallowWithAppProvider(<Button disabled />);
       expect(button.find('button').prop('disabled')).toBeTruthy();
     });
 
-    it('sets the disabled attribute on the link', () => {
+    it('disable with a url renders <a> without an href (as <a disabled> is invalid HTML)>', () => {
       const button = shallowWithAppProvider(
         <Button disabled url="http://google.com" />,
       );
-      expect(button.find(UnstyledLink).prop('disabled')).toBeTruthy();
+      expect(button.find('a').prop('href')).toBeFalsy();
     });
   });
 
   describe('loading', () => {
-    it('sets the disabled attribute on the button', () => {
+    it('loading without a url renders <button disabled>', () => {
       const button = shallowWithAppProvider(<Button loading />);
       expect(button.find('button').prop('disabled')).toBe(true);
     });
 
-    it('sets the disabled attribute on the link', () => {
+    it('loading with a url renders <a> without an href (as <a disabled> is invalid HTML)', () => {
       const button = shallowWithAppProvider(
         <Button loading url="http://google.com" />,
       );
-      expect(button.find(UnstyledLink).prop('disabled')).toBeTruthy();
+      expect(button.find('a').prop('href')).toBeFalsy();
     });
 
     it('renders a spinner into the button', () => {