fix(core:button): disabled/loading state #6183
Conversation
|
|
||
| .button-spinner { | ||
| --ring-color: #{$cds-global-color-construction-400}; | ||
| } |
There was a problem hiding this comment.
Was able to use the component properties and simplify the CSS
| await componentIsStable(anchorButton); | ||
| expect(anchor.querySelector('cds-button').readonly).toBe(true); | ||
| }); | ||
| describe('Button link', () => { |
There was a problem hiding this comment.
Moved this describe since it was nested in the describe above which made it a bit confusing what component it was referencing.
| ${this.loadingState === ClrLoadingState.loading | ||
| ? html`<cds-progress-circle .size=${this.size === 'sm' ? '12' : '18'} status="info"></cds-progress-circle>` | ||
| : ''} | ||
| ${this.loadingState === ClrLoadingState.default ? html`<slot></slot>` : ''} |
There was a problem hiding this comment.
Layout was simplified and didn't need the wrapper elements anymore
| </div> | ||
| `; | ||
| } | ||
|
|
There was a problem hiding this comment.
Icon button was the same template/logic duplicated as the inherited template
| import { errorStandardIcon } from '@cds/core/icon/shapes/error-standard.js'; | ||
| import { checkIcon } from '@cds/core/icon/shapes/check.js'; | ||
|
|
||
| ClarityIcons.addIcons(errorStandardIcon, checkIcon); |
There was a problem hiding this comment.
Moving the side effects to the register to follow the rest of the component register files
- update to initial disabled state after loading state has updated Signed-off-by: Cory Rylan <splintercode.cb@gmail.com>
5058c08 to
e827550
Compare
| it('should set spinner size to "12" if size "sm"', () => { | ||
| const templateResult = iconSpinner('sm'); | ||
| expect(templateResult.values.indexOf('12') > -1).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Moved to the rest of the loading state specs
| set loadingState(value: keyof typeof ClrLoadingState) { | ||
| // track prior disabled state to set prior value after button is re-enabled from a loading state | ||
| if (this._loadingState === ClrLoadingState.default) { | ||
| this._disabled = this.disabled; |
There was a problem hiding this comment.
The _disabled property was the key fix. To preserve the disabled state while not breaking or causing side effects we need to grab what the current disabled state is when updating from default to something else. This way when we go back to default we can use what the prior disabled state was.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
If a button was disabled prior to the loading state change it would attempt to revert to inital disabled state once loading was back to default. However the logic was not quite right and would break the loading state in subsequent updates.
Issue Number: #6129
What is the new behavior?
Now the loading state will disable the button when not default and when switch back to default set the disabled property the original value properly.
Does this PR introduce a breaking change?
Other information