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

disabled attribute doesn't work on <Button> #7905

Closed
3 tasks
BANOnotIT opened this issue Aug 1, 2023 · 3 comments
Closed
3 tasks

disabled attribute doesn't work on <Button> #7905

BANOnotIT opened this issue Aug 1, 2023 · 3 comments

Comments

@BANOnotIT
Copy link

Description

When I update @chakra-ui/button from 2.0.13 to 2.1.0 disable attribute doesn't work because of 0f37665. That's violation of SemVer because:

MINOR version when you add functionality in a backward compatible manner

Link to Reproduction

https://example.com

Steps to reproduce

No response

Chakra UI Version

2.8.0

Browser

No response

Operating System

  • macOS
  • Windows
  • Linux

Additional Information

No response

@BANOnotIT BANOnotIT changed the title [disabled] doesn't work on <Button> disabled attribute doesn't work on <Button> Aug 1, 2023
@ShawnFumo
Copy link

ShawnFumo commented Mar 12, 2024

Wanted to add that this ended up being a big issue for our company. We upgraded Chakra a few weeks ago, not realizing that this update broke most of our form buttons from disabling during dialog submissions. So we've had all kinds of data issues in production from people double-clicking the submit button, requiring time from people in the ops department, coordinating with multiple clients, etc.

Not only is it a semver issue, but it doesn't look like the change even got into the changelog? I searched through each version on https://chakra-ui.com/changelog/ and don't see the note from .changeset/empty-ravens-rescue.md in there anywhere. And if doing such a fundamental change, why not use Omit to remove the disabled property from those allowed on Button? That would have at least caused a compilation error after the update. Or even a warning in the console to say it is being ignored like what React does for missing keys on collections and such.

Yes, the docs don't seem to mention disabled at all, and yes we could have had more extensive Cypress tests to test for double-clicks, but if disabled and isDisabled are both listed as options in TypeScript and one has been working for at least a year, you don't expect that a simple library upgrade will cause your buttons to stop disabling with no warning.

Philzen added a commit to Philzen/chakra-ui that referenced this issue Apr 24, 2024
This reverts commit 0f37665.

Fixes chakra-ui#7269, chakra-ui#7816 and chakra-ui#7905 and also enables the only currently known
workaround for 	chakra-ui#7965. Removing the ability to use the disabled-prop
posed a breaking change, which is hereby fixed.
segunadebayo added a commit that referenced this issue May 8, 2024
* Revert "refactor: button disabled prop"

This reverts commit 0f37665.

Fixes #7269, #7816 and #7905 and also enables the only currently known
workaround for 	#7965. Removing the ability to use the disabled-prop
posed a breaking change, which is hereby fixed.

* docs: update

---------

Co-authored-by: Segun Adebayo <joseshegs@gmail.com>
@Philzen
Copy link
Contributor

Philzen commented May 10, 2024

@BANOnotIT #8462 has been merged, disabled prop on button works again, you can test that out in the LiveCoding-view here: https://chakra-ui-website-ptekgabqh-chakra-ui.vercel.app/docs/components/button

If satisfied, you may close this issue, the fix should be shipping with Chakra 2.8.3.

@BANOnotIT
Copy link
Author

It's great! But since this thing took so long to revert we just migrated to isDisabled so I don't have code to check whether it is fully compatible.

I suppose the issue can be closed

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 a pull request may close this issue.

3 participants