-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(overlay): add basic core of overlay #211
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
Conversation
|
||
/** | ||
* Service to create Overlays. Overlay are dynamically added pieces of floating UI, meant to be | ||
* used as low-level a building building block for other components. Dialogs, tooltips, menus, |
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.
A few typos:
"Overlay are" --> "Overlays are"
"used as low-level a building building block" --> "used as a low-level building block"
8beb2dd
to
9109f7b
Compare
|
||
/** | ||
* Service to create Overlays. Overlays are dynamically added pieces of floating UI, meant to be | ||
* used as a low-level building building block for other components. Dialogs, tooltips, menus, |
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.
Still has "building" twice?
LGTM, though still has that one typo if you want to fix |
@robertmesserle: the e2e is failing on this; https://travis-ci.org/angular/material2/builds/117214613 |
That means it’s working. He didn’t update the screenshot. Sent from my iPhone
|
I did update it. Seems like it fails to get the image. |
@jelbourn Ah, my bad - I was looking on my phone earlier and didn't see the screenshot in the changed files. Must've missed it. Anyway, hard to say what is happening, but this is one of the first things that made me want to push for going back to the server idea for screenshots: our method of manually handling |
Also, after looking at the screenshot URL, it's likely the issue I brought up earlier about PR's that live in other people's repos. The URL for your screenshot doesn't match the pattern of other screenshots we've dealt with so far, so manually handling the |
@robertmesserle another bullet point for your doc. |
9109f7b
to
376ffa8
Compare
376ffa8
to
f0e1273
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
R: @hansl @kara
This is the first step on the journey to overlays. I'll be doing it in as small of pieces as possible for ease of reviewing. The PR covers adding an overlay element to the DOM and attaching a portal to it. Future PRs will cover styling, sizing, positioning, and helper directives.