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(material-experimental/mdc-autocomplete): add panel animation #21525
Conversation
aa69cb5
to
f4d8a98
Compare
@@ -35,7 +36,8 @@ import { | |||
}, | |||
providers: [ | |||
{provide: MAT_OPTION_PARENT_COMPONENT, useExisting: MatAutocomplete} | |||
] | |||
], | |||
animations: [panelAnimation], |
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.
Should we have this on the non-MDC version too? I think that it may make the switch more difficult, because the timing in people's tests will change.
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.
My guess is adding it in the current one would be too hard to get in. We can try it at least in a separate PR
ca42c41
to
041b7cb
Compare
0047e3c
to
3007547
Compare
3007547
to
9844394
Compare
9844394
to
68032a1
Compare
CI issues should be resolved now, PTAL |
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.
LGTM with a couple of nits that aren't blocking.
transition(':enter, hidden => visible', [ | ||
group([ | ||
animate('0.03s linear', style({ opacity: 1 })), | ||
animate('0.12s cubic-bezier(0, 0, 0.2, 1)', style({ transform: 'scale(1)' })), |
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.
The indentation seems off here.
@@ -6,6 +6,7 @@ | |||
[ngClass]="_classList" | |||
[attr.aria-label]="ariaLabel || null" | |||
[attr.aria-labelledby]="_getPanelAriaLabelledby(formFieldId)" | |||
[@panelAnimation]="isOpen ? 'visible' : 'hidden'" |
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.
nit: Calling it "*Animation" is redundant since the @
means that it's an animation. We could simplify it to just "panel"?
…1525) * feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation Co-authored-by: Andrew Seguin <andrewjs@google.com> (cherry picked from commit 8aebd03)
…gular#21525) * feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation * fixup! feat(material-experimental/mdc-autocomplete): add panel animation Co-authored-by: Andrew Seguin <andrewjs@google.com>
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. |
No description provided.