snackbar: dynamic open #708
Conversation
Codecov Report
@@ Coverage Diff @@
## rc0.11.0_real #708 +/- ##
=================================================
- Coverage 95.14% 95.12% -0.03%
=================================================
Files 73 73
Lines 2907 2913 +6
Branches 444 447 +3
=================================================
+ Hits 2766 2771 +5
Misses 46 46
- Partials 95 96 +1
Continue to review full report at Codecov.
|
| if (prevProps.open !== open) { | ||
| if (open) { | ||
| this.foundation.open(); | ||
| } |
There was a problem hiding this comment.
shouldn't you call this.foundation.close() on an else clause?
There was a problem hiding this comment.
Correct...we need the this.foundation.close(reason). I suspect we would also want a closedReason prop that gets passed to reason in the close method. I'm open to other ideas too.
There was a problem hiding this comment.
we could use the prop with a default of the dismissed value from the mdc snackbar, what do you think?
There was a problem hiding this comment.
that seems reasonable to have a default, but we would still need to be able to override it.
There was a problem hiding this comment.
@lucasecdb sorry, my bad. Now I understand what you mean.
There was a problem hiding this comment.
The use case is if someone wants to programmatically close the snackbar before it automatically closes. I've attached a patch to add on to the screenshot tests.
There was a problem hiding this comment.
@moog16 I've applied the patch. I didn't event know this thing exists. thanks :)
I just called foundation.close with an empty argument if open prop is false.
There was a problem hiding this comment.
Can you add another prop for the reason? I'm thinking something like:
const {reason} = this.props;
...
this.foundation.close(reason)
There was a problem hiding this comment.
@moog16 Sure. Sorry for the late replies. I'll push the update you need in a moment.
|
fixes #707 |
| @@ -52,6 +52,13 @@ test('renders stacked actions', () => { | |||
| wrapper.unmount(); | |||
| }); | |||
|
|
|||
| test('#componentDidUpdate calls foundation.open if props.open is true', () => { | |||
There was a problem hiding this comment.
please add the opposite case for foundation.open = false
|
@aluminick just checking in to see what the status of this PR is. |
| @@ -38,6 +38,7 @@ export interface Props { | |||
| leading?: boolean; | |||
| stacked?: boolean; | |||
| open?: boolean; | |||
| reason?: string; | |||
There was a problem hiding this comment.
@aluminick please add this prop to the readme
There was a problem hiding this comment.
@aluminick looks good - just need to the readme update and we'll be good to go
|
Update golden.json snackbar entry to |
added missing componentDidUpdate for dynamic opening