vmware-clarity / core

Clarity is a scalable, accessible, customizable, open-source design system built with web components. Works with any JavaScript framework, created for enterprises, and designed to be inclusive.
https://clarity.design
MIT License
163 stars 42 forks source link

CdsButton with `undefined` `loadingState` renders incorrectly #186

Closed astorije-vmware closed 1 year ago

astorije-vmware commented 1 year ago

Describe the bug

When passing loadingState={undefined} (first button below), which should result in the same component as not providing any loading state at all, the button appears disabled with no content:

Screen Shot 2022-10-27 at 5 28 21 PM

How to reproduce

https://codesandbox.io/s/cds-button-loading-state-undefined-fuvizo

Expected behavior

The 3 buttons above should look and behave identically.

Versions

Clarity project:

Clarity version:

Framework:

Framework version:

React 18

ashleyryan commented 1 year ago

As we discussed offline, setting a property to undefined is not the same as omitting it. Since there are only 4 valid strings for loadingState, and undefined is not one of them, and since you have a workaround of doing <CdsButton loadingState={foo ? 'loading' : 'default'}>My Button</CdsButton> , instead of undefined, I'm going to close this as won't fix. It adds some additional complexity to the component that is easy enough to work around.

github-actions[bot] commented 1 year ago

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.