Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ActionList/Item] Fix disabling ActionList Item when url prop is passed #3979

Merged
merged 3 commits into from Feb 5, 2021

Conversation

CeeDeeBee
Copy link
Contributor

@CeeDeeBee CeeDeeBee commented Feb 5, 2021

WHY are these changes introduced?

Fixes an issue where ActionList Items will disable visually but not functionally when the url prop is passed.

WHAT is this pull request doing?

Sets the Item url and onClick to null if a url is passed and disabled is passed as true.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';

import {Page} from '../src';

export function Playground() {
  return (
    <Page
      title="Playground"
      actionGroups={[
        {
          title: 'Promote',
          actions: [
            {
              content: 'Share on Facebook',
              onAction: () => {
                // eslint-disable-next-line no-console
                console.log('test');
              },
              disabled: true,
              url: 'https://shopify.com',
            },
          ],
        },
      ]}
    >
      <p>Page content</p>
    </Page>
  );
}

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@ghost
Copy link

ghost commented Feb 5, 2021

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2021

🟡 This pull request modifies 3 files and might impact 58 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 58)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ActionList/components/Item/Item.tsx (total: 58)

Files potentially affected (total: 58)

🧩 src/components/ActionList/components/Item/tests/Item.test.tsx (total: 0)

Files potentially affected (total: 0)

@CeeDeeBee CeeDeeBee marked this pull request as ready for review February 5, 2021 00:19
@CeeDeeBee CeeDeeBee merged commit 8efeb4c into main Feb 5, 2021
@CeeDeeBee CeeDeeBee deleted the fix-action-list-item branch February 5, 2021 19:47
@ghost
Copy link

ghost commented Feb 5, 2021

🎉 Thanks for your contribution to Polaris React!

sylvhama pushed a commit that referenced this pull request Mar 26, 2021
…ed (#3979)

* Disable ActionList Item when url prop is passed

* Update UNRELEASED.md

* Update to set url and onClick to null + Add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants