fix(autocomplete): placeholder not animating on focus #3992
Conversation
googlebot
added
the
cla: yes
|
@kara LGTM, but you should take a look Screenshot failure seems unrelated (I think a gold didn't update when I approved and merged a previous PR). |
| if (this._inputContainer && this._inputContainer.floatPlaceholder === 'auto') { | ||
| this._inputContainer.floatPlaceholder = 'always'; | ||
| if (!shouldAnimate) { | ||
| this._inputContainer.floatPlaceholder = 'always'; |
There was a problem hiding this comment.
The reason why we set the placeholder to 'always' in this fashion is to avoid the placeholder jumping when an option is selected by the mouse (when previously no option was selected). Testing it locally, it looks like this change causes the placeholder to jump again. Can you take a look?
There was a problem hiding this comment.
I see what you mean, but in this case the only solution that both animates on focus and doesn't jump when clicking, would be to listen to the transitionend event on the placeholder and adding always after it is done. Wouldn't that be too much trouble for something this simple?
There was a problem hiding this comment.
Can you give it a shot? We can't really merge in a regression, especially for something as important as option selection.
There was a problem hiding this comment.
I had a PoC running, but it wasn't particularly elegant. I'll try to do something cleaner.
jelbourn
added
in progress
crisbeto
force-pushed
the
autocomplete-placeholder-animate
branch
2 times, most recently
from
e8a0d88
to
140f403
Compare
crisbeto
added
pr: needs review
and removed
in progress
|
@kara I've reworked this one as discussed, can you take another look? Note that the CI failure seems to be due to Saucelabs timing out. |
crisbeto
force-pushed
the
autocomplete-placeholder-animate
branch
2 times, most recently
from
c3056e4
to
43d46f6
Compare
|
@kara I've updated this one to work against the current master. Can you take another look? |
|
@crisbeto I am the worst. Can you re-base again? |
crisbeto
force-pushed
the
autocomplete-placeholder-animate
branch
from
43d46f6
to
f7a8f69
Compare
Fixes the placeholder not being animated on focus. **Note:** The `_handleFocus` and `openPanel` methods do pretty much the same (aside from the extra boolean), but I wanted to avoid having to pass the `$event` to `openPanel` since the event isn't really relevant to the API for opening the autocomplete programmatically. Fixes angular#5755.
crisbeto
force-pushed
the
autocomplete-placeholder-animate
branch
from
f7a8f69
to
ffd7e25
Compare
|
Rebased and set up to work with the form field. Can you take a look @kara? |
kara
added
pr: lgtm
action: merge
|
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. |
Fixes the placeholder not being animated on focus.
Note: The
_handleFocusandopenPanelmethods do pretty much the same (aside from the extra boolean), but I wanted to avoid having to pass the$eventtoopenPanelsince the event isn't really relevant to the API for opening the autocomplete programmatically.Fixes #5755.