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

fix(Tag): ignore click events when disabled #4792

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Nov 28, 2019

Closes #4791

This PR wraps the close icon in a button to utilize native button features (rather than spoofing a button with the SVG element) and ignores the onClick handler when the filter tag is disabled

Changelog

New

  • extend disabled tag styles for filter tag

Changed

Testing / Reviewing

Ensure click handlers are no longer fired when the tag filter is disabled and the component matches the visual spec (particularly the disabled state)

@netlify
Copy link

netlify bot commented Nov 28, 2019

Deploy preview for the-carbon-components ready!

Built with commit 60b4537

https://deploy-preview-4792--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Nov 28, 2019

Deploy preview for carbon-elements ready!

Built with commit 60b4537

https://deploy-preview-4792--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Nov 28, 2019

Deploy preview for carbon-components-react ready!

Built with commit 60b4537

https://deploy-preview-4792--carbon-components-react.netlify.com

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod!

Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Seems great to me, just a quick comment

packages/react/src/components/Tag/Tag.js Outdated Show resolved Hide resolved
@emyarod emyarod force-pushed the 4791-filter-tag-onclick-firing-when-disabled branch from 3d29bf3 to 38c846d Compare December 3, 2019 06:00
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

disabled tag looks good to me and is not clickable 👍

@stale
Copy link

stale bot commented Jan 5, 2020

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions.

@stale stale bot added the status: inactive Will close if there's no further activity within a given time label Jan 5, 2020
@asudoh asudoh removed the status: inactive Will close if there's no further activity within a given time label Jan 5, 2020
@emyarod emyarod force-pushed the 4791-filter-tag-onclick-firing-when-disabled branch 2 times, most recently from 6bf731b to d633147 Compare January 7, 2020 15:39
packages/components/src/components/tag/tag.hbs Outdated Show resolved Hide resolved
packages/components/src/components/tag/_tag.scss Outdated Show resolved Hide resolved
packages/react/src/components/Tag/Tag.js Outdated Show resolved Hide resolved
@emyarod emyarod force-pushed the 4791-filter-tag-onclick-firing-when-disabled branch from 5e88749 to b24f6c8 Compare January 8, 2020 00:39
@tw15egan
Copy link
Member

tw15egan commented Jan 8, 2020

@joshblack is this good to go now?

@emyarod emyarod force-pushed the 4791-filter-tag-onclick-firing-when-disabled branch from 9b25878 to 371ee11 Compare January 9, 2020 14:35
@joshblack joshblack merged commit 93baa1a into carbon-design-system:master Jan 10, 2020
joshblack added a commit to joshblack/carbon that referenced this pull request Jan 13, 2020
* fix(Tag): ignore click events when disabled

* test(Tag): remove warning

* fix(tag): move aria-label to button

* fix: add ellipses when width is constrained

* docs: fix example markup

* fix(tag): reintroduce padding due to recent markup change

* fix(Tag): remove unneeded click handler logic

* fix(tag): use disabled token

Co-authored-by: Josh Black <[email protected]>
@emyarod emyarod deleted the 4791-filter-tag-onclick-firing-when-disabled branch January 14, 2020 15:52
joshblack added a commit to joshblack/carbon that referenced this pull request Jan 14, 2020
* fix(Tag): ignore click events when disabled

* test(Tag): remove warning

* fix(tag): move aria-label to button

* fix: add ellipses when width is constrained

* docs: fix example markup

* fix(tag): reintroduce padding due to recent markup change

* fix(Tag): remove unneeded click handler logic

* fix(tag): use disabled token

Co-authored-by: Josh Black <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter tag fires click event when disabled
6 participants