fix(snack-bar): indicate in afterDismissed whether dismissal was a result of an action #9154

crisbeto · 2017-12-28T15:57:24Z

Adds a boolean to the MatSnackBarRef.afterDismissed result that indicates whether the dismissal came as a result of the user pressing the action button.

Fixes #9147.

crisbeto · 2017-12-28T15:59:12Z

src/lib/snack-bar/snack-bar-ref.ts

@@ -84,13 +88,18 @@ export class MatSnackBarRef<T> {
      this._onAction.complete();
    }

-    this._afterClosed.next();
-    this._afterClosed.complete();
+    this._afterDismissed.next(this._dismissedWithAction);


@josephperrott I'm also open to having a reason parameter on the dismiss method. I didn't go with it now, because it would mean that we'd need some magical string (e.g. 'action'), because the consumer doesn't usually have access to the action button on the SimpleSnackBar.

jelbourn · 2018-01-12T19:28:24Z

src/lib/snack-bar/snack-bar-ref.ts

+   * The stream emits a boolean that indicates whether the snack bar was dismissed
+   * using the action button.
+   */
+  afterDismissed(): Observable<boolean> {


Instead of making this a boolean stream, I'd rather add an event object so that we could add more information to it in the future.

crisbeto · 2018-01-13T11:22:30Z

Addressed the feedback @jelbourn.

jelbourn · 2018-01-16T22:46:07Z

src/lib/snack-bar/snack-bar-ref.ts

@@ -11,6 +11,12 @@ import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 import {MatSnackBarContainer} from './snack-bar-container';

+/** Event that is emitted when a snack bar is dismissed. */
+export interface MatSnackBarAfterDismiss {


How about just MatSnackBarDismiss or MatSnackBarDismissal? The "after" in the stream name makes sense because it relates to when the event is emitted, but there's nothing about the event object that's specific to it being at that time

Renamed to MatSnackBarDismiss.

josephperrott · 2018-01-18T17:36:02Z

src/lib/snack-bar/snack-bar-ref.ts

+/** Event that is emitted when a snack bar is dismissed. */
+export interface MatSnackBarDismiss {
+  /** Whether the snack bar was dismissied using the action button. */
+  dismissedByAction: boolean;


Should this be closedByAction since the method that would cause this to be true is closeWithAction()?

That's true, but the method that returns it is called afterDismissed. Either way it would be slightly confusing.

Maybe we should mark one of the terms as deprecated to fix it long term? I am good with either wording being removed, just as long as we create consistency.

My vote is for removing the "dismissed" wording and calling it "close" (also renaming the dismiss method). That way it's consistent with the dialog as well.

crisbeto · 2018-01-18T18:54:56Z

Done @josephperrott.

jelbourn · 2018-01-18T19:12:09Z

My opinion: dismiss is more appropriate for snackbar because you would, in conversation, use "dismiss" when the subject noun is "notification". I think the inconsistency is English language inconsistency leaking through (e.g. you get in a car but on a bus). I don't feel super strongly about it, but would lean on the side of not changing the API without a compelling technical reason.

…sult of an action Adds a boolean to the `MatSnackBarRef.afterDismissed` result that indicates whether the dismissal came as a result of the user pressing the action button. Fixes angular#9147.

crisbeto · 2018-01-19T20:49:53Z

Switched back to the dismiss terminology and deprecated closeWithAction. Can you take another look @josephperrott?

josephperrott

LGTM

angular-automatic-lock-bot · 2019-09-08T04:02:08Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added the pr: needs review label Dec 28, 2017

crisbeto assigned josephperrott Dec 28, 2017

crisbeto requested review from jelbourn and josephperrott as code owners December 28, 2017 15:57

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2017

crisbeto commented Dec 28, 2017

View reviewed changes

jelbourn reviewed Jan 12, 2018

View reviewed changes

crisbeto force-pushed the 9147/snack-bar-ref-after-dismissed branch from 40ac2ce to 1522681 Compare January 13, 2018 11:22

jelbourn reviewed Jan 16, 2018

View reviewed changes

crisbeto force-pushed the 9147/snack-bar-ref-after-dismissed branch from 1522681 to 045aefb Compare January 17, 2018 17:42

josephperrott reviewed Jan 18, 2018

View reviewed changes

crisbeto force-pushed the 9147/snack-bar-ref-after-dismissed branch from 045aefb to 846ba98 Compare January 18, 2018 18:54

fix(snack-bar): indicate in afterDismissed whether dismissal was a re…

e378824

…sult of an action Adds a boolean to the `MatSnackBarRef.afterDismissed` result that indicates whether the dismissal came as a result of the user pressing the action button. Fixes angular#9147.

crisbeto force-pushed the 9147/snack-bar-ref-after-dismissed branch from 846ba98 to e378824 Compare January 19, 2018 20:49

josephperrott approved these changes Jan 19, 2018

View reviewed changes

josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed pr: needs review labels Jan 19, 2018

jelbourn merged commit 829506d into angular:master Jan 23, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(snack-bar): indicate in afterDismissed whether dismissal was a result of an action #9154

fix(snack-bar): indicate in afterDismissed whether dismissal was a result of an action #9154

crisbeto commented Dec 28, 2017

crisbeto Dec 28, 2017

jelbourn Jan 12, 2018

crisbeto commented Jan 13, 2018

jelbourn Jan 16, 2018

crisbeto Jan 17, 2018

josephperrott Jan 18, 2018

crisbeto Jan 18, 2018 •

edited

Loading

josephperrott Jan 18, 2018

crisbeto Jan 18, 2018

josephperrott Jan 18, 2018

crisbeto commented Jan 18, 2018

jelbourn commented Jan 18, 2018 •

edited

Loading

crisbeto commented Jan 19, 2018

josephperrott left a comment

angular-automatic-lock-bot bot commented Sep 8, 2019

fix(snack-bar): indicate in afterDismissed whether dismissal was a result of an action #9154

fix(snack-bar): indicate in afterDismissed whether dismissal was a result of an action #9154

Conversation

crisbeto commented Dec 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto commented Jan 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto commented Jan 18, 2018

jelbourn commented Jan 18, 2018 • edited Loading

crisbeto commented Jan 19, 2018

josephperrott left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 8, 2019

crisbeto Jan 18, 2018 •

edited

Loading

jelbourn commented Jan 18, 2018 •

edited

Loading