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

Icon: disabled prop should disable onClick callback #3341

Closed
spencer-brown opened this issue Dec 14, 2018 · 3 comments
Closed

Icon: disabled prop should disable onClick callback #3341

spencer-brown opened this issue Dec 14, 2018 · 3 comments

Comments

@spencer-brown
Copy link

Bug Report

Based on past and open issues, it sounds like onClick callback execution has been updated to not fire when disabled === true for some but not all elements. I think Icons should not execute onClick callbacks when disabled === true to maintain parity with other elements (eg Button) which have been updated to maintain parity with HTML.

Steps

A clear and concise description of steps to reproduce the problem.

import React from 'react'
import { Icon } from 'semantic-ui-react'

const IconExampleDisabled = () => <Icon disabled name='users' onClick={() => console.log('hi') }/>

export default IconExampleDisabled

Click the icon.

Expected Result

When the icon is clicked, the onClick callback should not be called and "hi" should not be logged.

Actual Result

When the icon is clicked, "hi" is logged.

Version

0.82.3

Testcase

https://codesandbox.io/s/2o4qo44vrn

@welcome
Copy link

welcome bot commented Dec 14, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@layershifter layershifter changed the title Icon disabled prop should disable onClick callback Icon: disabled prop should disable onClick callback Dec 24, 2018
@danielsyang
Copy link
Contributor

Hello, can I work on this? It's going to be my first time contributing here :)

@layershifter
Copy link
Member

Yes, feel free to create a new PR 👍 Take a look on Button and it's tests for a reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants