Skip to content
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

bug(cdkTextareaAutosize): autosize scrolls parent to top at each key hit with ngZoneEventCoalescing enabled #23834

Open
michael-hein opened this issue Oct 26, 2021 · 8 comments
Labels
area: cdk/text-field P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@michael-hein
Copy link

Reproduction

  1. open in chrome https://stackblitz.com/edit/angular-dqeq92?file=src%2Fapp%2Fdialog-content-example.html
  2. type a few line breaks or texts into the textarea until the the div's scrollbar appear
  3. scroll to the bottom and type more line breaks or text

Expected Behavior

The scroll position of the div should not be changed.

Actual Behavior

After each key hit the div gets scrolled to top. So its not possible to see what is currently typed.

Environment

  • Angular: 12.2.11
  • CDK/Material: 12.2.9
  • Browser(s): Chrome, Edge (Chromium), Opera
  • Operating System: Windows

When ngZoneEventCoalescing is set to false, it works in Chrome as expected.
It seems like its related to #23233. In firefox it works as expected.

When the firefox work around in the autosize Directive is used for chrome too, then it works fine. But I'm not really know, if it has some other side effects when using the work around for chrome too.

const needsMarginFiller = isFirefox && this._hasFocus;
const measuringClass = isFirefox
? 'cdk-textarea-autosize-measuring-firefox'
: 'cdk-textarea-autosize-measuring';
// In some cases the page might move around while we're measuring the `textarea` on Firefox. We
// work around it by assigning a temporary margin with the same height as the `textarea` so that
// it occupies the same amount of space. See #23233.
if (needsMarginFiller) {
element.style.marginBottom = `${element.clientHeight}px`;
}

@michael-hein michael-hein added the needs triage This issue needs to be triaged by the team label Oct 26, 2021
@crisbeto
Copy link
Member

I was able to reproduce the issue, but I don't know if we can apply the margin trick from Firefox, because we use a different class while the textarea is being measured (see https://github.com/angular/components/blob/master/src/cdk/text-field/_index.scss#L9). The reason it works for Firefox is that the element also has height: 0.

@michael-hein
Copy link
Author

I tried it out in another stackblitz with a custom autosize directive where the isFirefox variable is set to true. So that the firefox class and the margin trick gets used.
https://stackblitz.com/edit/angular-dqeq92-vdymoj?file=src%2Fapp%2Fcustom-autosize.ts

    const isFirefox = (true || this._platform.FIREFOX);
    const needsMarginFiller = isFirefox && this._hasFocus;
    const measuringClass = isFirefox
      ? 'cdk-textarea-autosize-measuring-firefox'
      : 'cdk-textarea-autosize-measuring';

    // In some cases the page might move around while we're measuring the `textarea` on Firefox. We
    // work around it by assigning a temporary margin with the same height as the `textarea` so that
    // it occupies the same amount of space. See #23233.
    if (needsMarginFiller) {
      element.style.marginBottom = `${element.clientHeight}px`;
    }

then the scrolling works better, so its not getting scrolled to top.

I also tried it out without the firefox class and in this case it also works without it.

@crisbeto
Copy link
Member

The question is whether we can safely apply the Firefox class everywhere. The .scss file I linked above has some comments that the height: 0 causes issues in Chrome and IE. We don't have to worry about IE anymore, but Chrome could still be a problem.

@michael-hein
Copy link
Author

do we need to use the firefox class?
For Chrome it works without setting the height: 0 so there is no need to use it, or?

@crisbeto
Copy link
Member

Wouldn't that cause content below the input to jump? What would happen is that the textearea will become height: auto and margin-top: {{previousHeight}}.

@crisbeto crisbeto added area: cdk/text-field P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs triage This issue needs to be triaged by the team labels Nov 7, 2021
@ptu14
Copy link

ptu14 commented Aug 7, 2022

I also have a problem on Chrome with that. Creating my own copy of the directive and disabling Firefox checking (actually the needsMarginFiller condition) solved the problem for me.

Wouldn't that cause content below the input to jump? What would happen is that the textearea will become height: auto and margin-top: {{previousHeight}}.

@crisbeto Correct me if I'm wrong, but I think you could disable this checking and content will not jump, because these changes take place in one cycle of the event loop.

@idimash
Copy link

idimash commented Dec 21, 2023

I still have a problem on Chrome
Any update on this?

@ameramayreh
Copy link

I have fixed this by temporary set the parent's min-height to its offsetHeight before adding the measuring class:

    const previousParentMinHeight = element.parentElement.style.minHeight;
    element.parentElement.style.minHeight = `${element.parentElement.offsetHeight}px`;
    // Reset the textarea height to auto in order to shrink back to its default size.
    // Also temporarily force overflow:hidden, so scroll bars do not interfere with calculations.
    element.classList.add(measuringClass);
    // The measuring class includes a 2px padding to workaround an issue with Chrome,
    // so we account for that extra space here by subtracting 4 (2px top + 2px bottom).
    const scrollHeight = element.scrollHeight;
    element.classList.remove(measuringClass);
    element.parentElement.style.minHeight = previousParentMinHeight;

It worked perfectly with me.
Is there any concerns about this fix?

If not, I can submit a pull request for it.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cdk/text-field P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

5 participants