-
Notifications
You must be signed in to change notification settings - Fork 7
Add support for disabled and loading states to polaris-setting-toggle #199
Add support for disabled and loading states to polaris-setting-toggle #199
Conversation
docs/setting-toggle.md
Outdated
@@ -14,6 +14,7 @@ Inline usage: | |||
enabled=enabled | |||
action=(hash | |||
text="Toggle it!" | |||
loading=isToggleSettingRunning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we document disabled
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about it but I'm not sure there's any real value
`); | ||
|
||
let button = find(settingActionButtonSelector); | ||
assert.ok( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could also be written as
assert.dom(settingActionButtonSelector).hasClass('Polaris-Button--loading', 'button is in loading state when loading is true');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could... but I want to get this out quickly so wasn't going to update the test file to use the newer things 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you need this, let's skip updating test now, though documenting properties in the component (if not there already) would be nice before shipping
Just found that the
disabled
andloading
options inpolaris-setting-toggle
'saction
hash weren't supported, so added them in.