fix(autosize): not updating when window is resized #8619
Conversation
googlebot
added
the
cla: yes
src/lib/input/autosize.ts
Outdated
| } | ||
| } | ||
|
|
||
| ngOnDestroy() { | ||
| this._resize.unsubscribe(); |
There was a problem hiding this comment.
Haven't we been switching to the _destoryed.next() style?
src/lib/input/autosize.ts
Outdated
|
|
||
| this._resize = this._ngZone.runOutsideAngular(() => { | ||
| return fromEvent(window, 'resize') | ||
| .pipe(debounceTime(10)) |
There was a problem hiding this comment.
wouldn't auditTime be more appropriate here?
There was a problem hiding this comment.
I don't think so. We want to prevent this from happening if we get a bunch of resize events close to each other.
There was a problem hiding this comment.
But in this case the rest of the page will be smoothly resizing and the textarea will just jump to the correct size at the end right? Wouldn't it be better to have it also smoothly resize?
There was a problem hiding this comment.
I think that the delay is so small that people won't really notice it if they're flipping their phone. The only ones that would would be developers, but I'd prioritize less repaints to dev satisfaction while resizing the window.
There was a problem hiding this comment.
the delay is small but if you get a resize event every 9ms for 1s, doesn't that mean the resize won't actually trigger until after 1s? (or I'm misunderstanding how debounceTime works)
"window of time required to wait for emission silence before emitting the most recent source value" (from http://reactivex.io/rxjs/class/es6/Observable.js~Observable.html#instance-method-debounceTime)
There was a problem hiding this comment.
It does, but the way this usually plays out is that the browser either fires a few of them at a time if you change your screen orientation, or one per pixel if you're resizing it manually. It's really hard to move your browser resize handle at a constant 1px per 9ms since it's about the speed of human reaction time.
There was a problem hiding this comment.
auditTime seems like a more natural fit for this though, since it will give us the most recent value every 10ms (or we could up it to 16ms which is ~60fps)
There was a problem hiding this comment.
Makes sense, I've switched it.
src/lib/input/autosize.ts
Outdated
| /** | ||
| * Resize the textarea to fit its content. | ||
| * @param force Whether to force a height recalculation. By default the height will be | ||
| * recalculated only if the value changed since the last call. |
crisbeto
force-pushed
the
8610/autosize-resize
branch
from
23d4f08 to
4328501
Compare
|
Addressed the feedback @mmalerba. |
crisbeto
force-pushed
the
8610/autosize-resize
branch
from
4328501 to
1fe3e53
Compare
mmalerba
added
pr: lgtm
action: merge
|
I think this is technically breaking due to the addition of If we want to get it in sooner we could make the |
crisbeto
force-pushed
the
8610/autosize-resize
branch
from
1fe3e53 to
03c73b2
Compare
|
Made it optional, although I'm not sure how common it would be to extend this directive in particular. |
crisbeto
force-pushed
the
8610/autosize-resize
branch
from
03c73b2 to
aec5a66
Compare
| constructor( | ||
| private _elementRef: ElementRef, | ||
| private _platform: Platform, | ||
| private _ngZone?: NgZone) {} |
There was a problem hiding this comment.
nit: add TODO to make non-optional at next major
Fixes the autosize textarea not recalculating its height when the viewport size has changed. Fixes angular#8610.
crisbeto
force-pushed
the
8610/autosize-resize
branch
from
aec5a66 to
1a11ffa
Compare
Fixes the autosize textarea not recalculating its height when the viewport size has changed. Fixes #8610.
|
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 autosize textarea not recalculating its height when the viewport size has changed.
Fixes #8610.