Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(snackbar): allow snack bar to be positioned on the page #4045

Merged
merged 1 commit into from
Sep 11, 2017

Conversation

josephperrott
Copy link
Member

For #3577

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 12, 2017
@jelbourn jelbourn self-assigned this Apr 12, 2017
@jelbourn jelbourn self-requested a review April 12, 2017 23:44
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, I think I would want to make this more general before adding more positions

@jelbourn jelbourn added in progress This issue is currently in progress and removed pr: needs review labels Apr 14, 2017
@josephperrott josephperrott force-pushed the snackbar branch 5 times, most recently from 8932bc7 to 6be0d6d Compare April 26, 2017 05:38
@xtianus79
Copy link

@jelbourn and @josephperrott so this is going to be only left of center at first?

@josephperrott josephperrott force-pushed the snackbar branch 3 times, most recently from b1a462c to d9f16e9 Compare April 27, 2017 23:33
@josephperrott josephperrott changed the title Allow snack bar position to be set to left or center. feat(snackbar): allow snack bar to be positioned on the page Apr 28, 2017
@josephperrott
Copy link
Member Author

@jelbourn ready for you to take a look

@josephperrott josephperrott removed the in progress This issue is currently in progress label May 11, 2017
@@ -1,6 +1,11 @@
import {ViewContainerRef} from '@angular/core';
import {AriaLivePoliteness} from '../core';

export interface MdSnackBarPosition {
horizontal: string;
vertical: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types for these should be a string literal enumeration

@@ -19,4 +24,10 @@ export class MdSnackBarConfig {

/** Extra CSS classes to be added to the snack bar container. */
extraClasses?: string[];

/** The position to place the snack bar in the view, either 'left' or 'center'. */
position?: MdSnackBarPosition = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of want this to be two separate properties (verticalPosition and horizontalPosition)- it gives you the freedom to set just one if you want (easier defaults). Do you have a reason for making it a separate object?

max-width: $mat-snack-bar-max-width;
min-width: $mat-snack-bar-min-width;
padding: $mat-snack-bar-padding;
// Initial transformation is applied to start snack bar out of view.
transform: translateY(100%);

&.md-snack-bar-center {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add description comments for when these classes are applied / what they're used for

.bottom('0');
let positionStrategy = this._overlay.position().global();
switch (config.position.horizontal) {
case 'left':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be start and end instead of left and right, taking the RTL/LTR state into account

case 'bottom':
positionStrategy.bottom('0');
break;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need unit test for using the appropriate position based on the config and ltr/rtl state


if (this.snackBarConfig.position.vertical === 'top') {
this._renderer.setElementClass(this._elementRef.nativeElement, 'md-snack-bar-top', true);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need unit tests for setting these css classes when appropriate

@josephperrott josephperrott force-pushed the snackbar branch 4 times, most recently from 08f89e3 to a747c07 Compare May 13, 2017 00:17
Copy link

@soorireddy soorireddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the changes , can we get this merged ASAP

@xtianus79
Copy link

@jelbourn & @josephperrott would this just be in the next build or will be available on master branch anytime in the future

@mmalerba mmalerba added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Jul 7, 2017
@mmalerba
Copy link
Contributor

mmalerba commented Jul 7, 2017

please rebase

@xtianus79
Copy link

Is this going to make it into the next build? I'm really waiting for this feature. @josephperrott

@koumatsumoto
Copy link
Contributor

I hope next build come with this changes.

@xtianus79
Copy link

This release is taking longer than expected. Any idea when the next release is?

@josephperrott
Copy link
Member Author

@jelbourn rebased and ready to be looked at.

@jelbourn jelbourn added action: merge The PR is ready for merge by the caretaker and removed presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged labels Aug 28, 2017
@jelbourn
Copy link
Member

jelbourn commented Sep 1, 2017

@josephperrott can you rebase again?

@josephperrott
Copy link
Member Author

@jelbourn Rebased and ready to go.

@b4z81
Copy link

b4z81 commented Sep 11, 2017

+1 merge please

@mmalerba mmalerba merged commit 426324b into angular:master Sep 11, 2017
josephperrott added a commit to josephperrott/components that referenced this pull request Sep 15, 2017
@xtianus79
Copy link

@jelbourn && @josephperrott did this end up making it into the latest build? If not, is there a new holdup for this feature?

@jelbourn
Copy link
Member

This should be in beta.11

@xtianus79
Copy link

@jelbourn yea!!! I looked in the release notes and didn't see it.

@ORESoftware
Copy link
Contributor

How can we change the position/location of snackbar?

@CDDelta
Copy link

CDDelta commented Feb 18, 2018

@ORESoftware have a look at this https://angular-vv8kca.stackblitz.io.

@ORESoftware
Copy link
Contributor

ah yes thanks

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.