-
Notifications
You must be signed in to change notification settings - Fork 6.8k
refactor(overlay): allow for object to be passed to the OverlayState constructor #6615
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,10 @@ import {NoopScrollStrategy} from './scroll/noop-scroll-strategy'; | |
*/ | ||
export class OverlayState { | ||
/** Strategy with which to position the overlay. */ | ||
positionStrategy: PositionStrategy; | ||
positionStrategy?: PositionStrategy; | ||
|
||
/** Strategy to be used when handling scroll events while the overlay is open. */ | ||
scrollStrategy: ScrollStrategy = new NoopScrollStrategy(); | ||
scrollStrategy?: ScrollStrategy = new NoopScrollStrategy(); | ||
|
||
/** Custom class to add to the overlay pane. */ | ||
panelClass?: string | string[] = ''; | ||
|
@@ -53,6 +53,12 @@ export class OverlayState { | |
/** The direction of the text in the overlay panel. */ | ||
direction?: Direction = 'ltr'; | ||
|
||
constructor(state?: OverlayState) { | ||
if (state) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you do this? extendObject(this, state); (from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I forgot that was in material. |
||
Object.keys(state).forEach(key => this[key] = state[key]); | ||
} | ||
} | ||
|
||
// TODO(jelbourn): configuration still to add | ||
// - focus trap | ||
// - disable pointer events | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since all of the options are optional now, could you just change most places to, e.g.,
?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I did. The
position
in this particular case is different, because we want to save a reference to it before we pass it to theOverlayRef
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant omitting the
new OverlayState(
part and just having the object literalThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, gotcha. Instantiating the class lets us apply the default options for things like the direction, backdrop class and scroll strategy.