fix(snack-bar): use mat-button like styling in simple snack bar #7506

josephperrott · 2017-10-03T20:21:20Z

crisbeto · 2017-10-03T20:24:30Z

src/lib/snack-bar/simple-snack-bar.scss

@@ -16,6 +16,14 @@ $mat-snack-bar-button-margin: 48px !default;
  background: none;


It doesn't let me set a comment on the proper line, but you should be able to get rid of the @include mat-button-reset a little bit above.

crisbeto · 2017-10-03T20:24:58Z

src/lib/snack-bar/simple-snack-bar.scss

@@ -16,6 +16,14 @@ $mat-snack-bar-button-margin: 48px !default;
  background: none;
  flex-shrink: 0;
  margin-left: $mat-snack-bar-button-margin;
+  line-height: 20px;


Is this used for centering? Couldn't we use flexbox instead?

See other comment

crisbeto · 2017-10-03T20:25:59Z

src/lib/snack-bar/simple-snack-bar.scss

+
+  .mat-button-ripple,
+  .mat-button-focus-overlay {
+    height: 36px;


Why does this have to override the height and top? AFAIK the overlay and ripple are supposed to be width: 100% and height: 100% of the button?

The issue that I am seeing is that the size of the button is supposed to fall outside of the content box of the mat-simple-snackbar.

The snackbar provides 20px of space in a single line, but the button is supposed to be 36px. The "best" solution I think would actually be to move the padding-like spacing from the container to the mat-simple-snackbar. Thoughts?

You should be able to work around that by using a negative margin. E.g. if you remove the size and position overrides of the ripple and focus overlay, as well as the line-height: 20px from the button and add something like margin: -14px -24px -14px 48px you can get the button to overlap the container padding. You might have to tweak the spacing a bit.

josephperrott · 2017-10-06T18:38:43Z

@crisbeto Take another look, I ended up using margins and flex to handle both the single and multi line cases.

crisbeto · 2017-10-06T18:40:33Z

src/lib/snack-bar/simple-snack-bar.scss

  flex-shrink: 0;
-  margin-left: $mat-snack-bar-button-margin;
+  justify-content: space-around;
+  margin: -8px 0 -8px $mat-snack-bar-button-margin;


Use the variable here?

I can use a variable, however they are different margins so to speak.

The top and bottom margins are -8px so that the total line height adds back up to 36px (20 + 8 + 8). The left and right margins are for spacing within the container according to spec.

Should I create another variable? Or comment to explain it?

Having another variable should be fine.

crisbeto · 2017-10-06T18:40:56Z

src/lib/snack-bar/simple-snack-bar.scss

+
+  button {
+    flex: 1;
+    max-height: 36px;


Move this into a variable or add a comment about where it's coming from?

crisbeto

LGTM

jelbourn · 2017-10-26T17:37:01Z

Investigating whether we want to add a ripple instead of bringing in all of MatButton

josephperrott · 2017-10-26T20:54:09Z

After investigation it was determined that adding ripple manually would essentially amount to recreating MatButton within snack bar.

jelbourn

LGTM

jelbourn · 2018-01-03T23:01:34Z

@josephperrott this needs to update the bazel rule now

jelbourn · 2018-01-03T23:02:01Z

Caretaker note: needs a simple build file update

angular-automatic-lock-bot · 2019-09-08T14:01:30Z

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.}

josephperrott requested a review from crisbeto October 3, 2017 20:21

josephperrott requested a review from jelbourn as a code owner October 3, 2017 20:21

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

josephperrott added the pr: needs review label Oct 3, 2017

crisbeto reviewed Oct 3, 2017

View reviewed changes

josephperrott force-pushed the snackbar-button branch from 272786d to fec58ba Compare October 6, 2017 18:37

crisbeto reviewed Oct 6, 2017

View reviewed changes

josephperrott force-pushed the snackbar-button branch 2 times, most recently from 6385e79 to a8555cb Compare October 6, 2017 20:49

crisbeto approved these changes Oct 6, 2017

View reviewed changes

crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Oct 6, 2017

josephperrott force-pushed the snackbar-button branch 2 times, most recently from 89c2c0c to 7aa229d Compare October 6, 2017 22:05

jelbourn removed the action: merge The PR is ready for merge by the caretaker label Oct 26, 2017

josephperrott force-pushed the snackbar-button branch 2 times, most recently from d3b30c1 to 19aace8 Compare October 26, 2017 17:41

jelbourn approved these changes Oct 26, 2017

View reviewed changes

jelbourn added the action: merge The PR is ready for merge by the caretaker label Oct 26, 2017

josephperrott changed the title ~~fix(snack-bar): use mat-button in simple snack bar~~ fix(snack-bar): use mat-button like styling in simple snack bar Nov 2, 2017

josephperrott force-pushed the snackbar-button branch 3 times, most recently from 49d8ceb to 0d99552 Compare November 20, 2017 16:13

josephperrott force-pushed the snackbar-button branch 3 times, most recently from 731ef9b to c5b33b6 Compare December 6, 2017 21:06

josephperrott added the target: minor This PR is targeted for the next minor release label Dec 6, 2017

andrewseguin added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Dec 13, 2017

josephperrott force-pushed the snackbar-button branch 2 times, most recently from c812578 to d4e4f5b Compare January 8, 2018 18:19

josephperrott added the action: merge The PR is ready for merge by the caretaker label Jan 8, 2018

josephperrott force-pushed the snackbar-button branch 2 times, most recently from 9328f7e to 378e5e2 Compare March 7, 2018 21:25

josephperrott added the P2 The issue is important to a large percentage of users, with a workaround label Mar 8, 2018

josephperrott force-pushed the snackbar-button branch 3 times, most recently from 2390328 to 9bf6303 Compare March 28, 2018 16:44

Use mat-button in simple snack bar.

da4b80a

josephperrott force-pushed the snackbar-button branch from 9bf6303 to da4b80a Compare March 29, 2018 21:21

josephperrott merged commit b47bfaf into angular:master Mar 29, 2018

josephperrott deleted the snackbar-button branch March 29, 2018 21:53

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

		@@ -16,6 +16,14 @@ $mat-snack-bar-button-margin: 48px !default;
		background: none;

fix(snack-bar): use mat-button like styling in simple snack bar #7506

fix(snack-bar): use mat-button like styling in simple snack bar #7506

Uh oh!

Conversation

josephperrott commented Oct 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephperrott commented Oct 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Oct 26, 2017

Uh oh!

josephperrott commented Oct 26, 2017

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Jan 3, 2018

Uh oh!

jelbourn commented Jan 3, 2018

Uh oh!

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

Uh oh!

Uh oh!