Adds a new deprovision button and it's resource-container wrapper #245
Conversation
This adds a deprovision button based on the code for the provision button. This button can be used standalone and will fetch the resource id if not provided, or as a `manifold-resource-deprovision` component that will get it's data from the `resource-container` component. Work is related to manifoldco/engineering#8688
Codecov Report
@@ Coverage Diff @@
## master #245 +/- ##
==========================================
+ Coverage 57.98% 58.77% +0.78%
==========================================
Files 39 40 +1
Lines 883 946 +63
Branches 204 222 +18
==========================================
+ Hits 512 556 +44
- Misses 362 379 +17
- Partials 9 11 +2
Continue to review full report at Codecov.
|
| render() { | ||
| return ( | ||
| <button | ||
| type="submit" |
There was a problem hiding this comment.
should this be a submit? doesn't a submit usually go inside a form? or are we expecting to use this in a form in association with a text input
There was a problem hiding this comment.
All our data buttons are submits, not sure why 🤔 @DangoDev do you have an idea?
There was a problem hiding this comment.
Yeah typically these buttons are in forms with other bits of form data (e.g. a text input). type="submit" enables users to hit enter from the name field, which is preferable.
But if there’s not an input it doesn’t matter as much. But maybe the ability to submit a form is nice, in general?
| <manifold-data-deprovision-button resourceId={state.data.id} resourceName={state.data.label}> | ||
| <manifold-forward-slot> | ||
| <slot /> | ||
| </manifold-forward-slot> |
There was a problem hiding this comment.
What does manifold-forward-slot do?
There was a problem hiding this comment.
It allows this deprovision button to "forward" its slot into the internal data deprovision button so this is possible:
<manifold-resource-deprovision>
Content
</manifold-resource-deprovision>There was a problem hiding this comment.
Sam made this. This is just a nice utility helper to pass named slots several layers through components. Otherwise it’s just a little more brittle.
| <slot /> | ||
| </manifold-forward-slot> | ||
| </manifold-data-deprovision-button> | ||
| )} |
There was a problem hiding this comment.
seems like there would be a cleaner way to do this ternary rather than duplicating the markup. 🤔
something like this maybe?
...
<manifold-data-deprovision-button
resourceId={state.data ? state.data.id : undefined}
resourceName={state.data ? state.data.label : undefined}
loading={state.loading}
>
...There was a problem hiding this comment.
I find the ternary more readable, but I'm open to changing if you think it's a blocker 🤷♂
There was a problem hiding this comment.
I'll defer to @DangoDev for a ruling on that. I don't feel too strongly about it
There was a problem hiding this comment.
Dave’s way is clearer to my brain, but it’s a minor thing. I don’t feel too strongly about it either. 🍌
| </manifold-data-provision-button> | ||
| --- | ||
|
|
||
| # 🔒 Derovision Button |
| DOM][shadow-dom] encapsulation for ease of use, we figured this component | ||
| should be customizable. As such, style it however you’d like! We recommend | ||
| attaching styles to a parent element using any CSS-in-JS framework of your | ||
| choice, or plain ol’ CSS. |
| return; | ||
| } | ||
|
|
||
| this.resourceId = resources[0].id; |
There was a problem hiding this comment.
Ah I see what you’re doing here. Yeah this is strange, but I can’t think of a better way.
There was a problem hiding this comment.
Overall though I think it’s nice functionality allowing resource-name or resource-id to be used 👍
…o gui/8688-deprovision-button
Work is related to manifoldco/engineering#8688
Reason for change
This adds a deprovision button based on the code for the provision button. This button can be used standalone and will fetch the resource id if not provided, or as a
manifold-resource-deprovisioncomponent that will get it's data from theresource-containercomponent.Testing
Test with storybook