Skip to content

Conversation

@awebartisan
Copy link
Contributor

Adds type=button attribute to Clickable Tag.

WHY are these changes introduced?

Fixes #2894

WHAT is this pull request doing?

Adding type="button" attribute to clickable Tag.

Adds type=button attribute to Clickable Tag and solves Shopify#2894
@ghost
Copy link

ghost commented Apr 4, 2020

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

@ghost ghost added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 4, 2020
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this, @awebartisan!!

Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need you to sign the CLA and move the attributes on the button element to their own lines to fix the lint error causing the build to fail. If you happen to use VSCode as your text editor, add the Prettier extension and make any edit to the file then save and Prettier will autofix for you.

Screen Shot 2020-04-06 at 12 39 10 PM

Moved attributes of button on their own lines.

Co-Authored-By: Chloe Rice <chloerice@users.noreply.github.com>
@awebartisan
Copy link
Contributor Author

Hi @chloerice, I made the commit before signing the CLA. Is there any way I can re-run the CLA check?

Thanks.

@chloerice
Copy link
Member

chloerice commented Apr 9, 2020

Hi @chloerice, I made the commit before signing the CLA. Is there any way I can re-run the CLA check?

Thanks.

Hey @awebartisan! You'll need to add an entry to UNRELEASED.md under Bug fixes to pass the change log check.

- Fixed `Tag` submitting forms when `onClick` is set ([#2895](https://github.com/Shopify/polaris-react/pull/2895))

The linter is also complaining about extra whitespace after <button => <button.

Pull down the commit made through the PR suggestion, fix that extra whitespace, and add the change log entry, all the checks will run again after you push your changes up.

@ghost ghost removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 15, 2020
@chloerice chloerice dismissed their stale review April 17, 2020 20:49

Requested changes made ✅

Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for contributing @awebartisan 🙏!!

@chloerice
Copy link
Member

Hey @awebartisan, everything is good to go but a few of the PR checks froze up. Would you mind pushing up from your branch again to re-trigger the checks?

@awebartisan
Copy link
Contributor Author

Hey @chloerice , I would need to make some changes in order to be able to commit/push again. Can you suggest some?

Thanks 🙏🏼

@chloerice
Copy link
Member

chloerice commented Apr 20, 2020

Hey @chloerice , I would need to make some changes in order to be able to commit/push again. Can you suggest some?

Thanks 🙏🏼

Hey @awebartisan, my teammate informed me that those unrun checks don't run on public contributions, so all I need to do is resolve the conflicts in UNRELEASED.md and I'll be able to merge for you 😊

@awebartisan
Copy link
Contributor Author

Thank you 🙏🏼 I thought I messed up badly 😞Thank you for bearing with me.

Looking forward to merge of my first pull request in Shopify ecosystem 😍

@chloerice chloerice merged commit 5abaf9d into Shopify:master Apr 20, 2020
@ghost
Copy link

ghost commented Apr 20, 2020

🎉 Thanks for your contribution to Polaris React!

@juanpprieto
Copy link

juanpprieto commented Apr 29, 2022

Unfortunately this change makes the type prop to be hardcoded as "button". Frameworks such as https://github.com/remix-run/remix use native form submit flows in creative ways to communicate between the client and the server in a full React context. Please allow the type prop to be overwritten when passed in.

Thanks!
JP

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.

[Tag component] Add the type=button attribute to avoid form submissions

3 participants