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

Textarea: resize immediately upon receiving resize event #3463

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

JulianNymark
Copy link
Contributor

@JulianNymark JulianNymark commented Dec 16, 2024

Description

got feedback on buggy behaviour of textarea "jumping" inside a modal (upon opening).

Component Checklist 📝

  • JSDoc
  • Examples
  • Documentation / Decision Records
  • Storybook
  • Style mappings (@navikt/core/css/config/_mappings.js)
  • Component tokens (@navikt/core/css/tokens.json)
  • CSS class deprecations (@navikt/aksel-stylelint/src/deprecations.ts)
  • Exports (@navikt/core/react/src/index.ts and @navikt/core/react/package.json)
  • New component? CSS import (@navikt/core/css/index.css)
  • Breaking change? Update migration guide. Consider codemod.
  • Changeset (Format: <Component>: <gitmoji?> <Text>. E.g. "Button: ✨ Add feature xyz.")

Copy link

changeset-bot bot commented Dec 16, 2024

🦋 Changeset detected

Latest commit: c4243fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@navikt/ds-react Patch
@navikt/ds-css Patch
@navikt/ds-tokens Patch
@navikt/ds-tailwind Patch
@navikt/aksel-icons Patch
@navikt/aksel Patch
@navikt/aksel-stylelint Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 16, 2024

Storybook demo

6d4006631 | 91 komponenter | 135 stories

@JulianNymark
Copy link
Contributor Author

Thinking this could be solved via CSS set on the parent of the element maybe? This could be due to when the resize event is fired, and it happens after the element is added to the dom => gets resized to the new size?

https://github.com/navikt/aksel/blob/main/@navikt/core/react/src/util/TextareaAutoSize.tsx#L187

@HalvorHaugan
Copy link
Contributor

The solution is to run handleResize() on the leading edge of the timeout. Could either add this feature to our current debounce() function, or switch to lodash.

Here is an improved version of our current function in case we want to continue using that:

"use client";
// https://github.com/mui/material-ui/blob/master/packages/mui-utils/src/debounce.js
export default function debounce<T extends unknown[]>(
  func: (...args: T) => void,
  wait = 166,
  leading = false,
) {
  let timeout: ReturnType<typeof setTimeout> | undefined;
  function debounced(this: any, ...args: T) {
    const later = () => {
      timeout = undefined;
      func.apply(this, args);
    };
    if (!timeout && leading) {
      later();
    }
    clearTimeout(timeout);
    timeout = setTimeout(later, wait);
  }

  debounced.clear = () => {
    clearTimeout(timeout);
  };

  return debounced;
}

I think we should add a comment in TextareaAutoSize.tsx explaining why this is needed. Something like this:

Invoke on leading edge of timeout to avoid delayed resize when toggling visibility (typically modals) (scrollHeight is 0 when hidden)

@JulianNymark JulianNymark marked this pull request as ready for review December 19, 2024 12:51
JulianNymark and others added 2 commits December 19, 2024 14:11
Co-authored-by: Halvor Haugan <halvor.haugan@nav.no>
@HalvorHaugan HalvorHaugan changed the title textarea inside modal Textarea: resize immediately upon receiving resize event Dec 19, 2024
@HalvorHaugan HalvorHaugan merged commit a2b3936 into main Dec 19, 2024
4 checks passed
@HalvorHaugan HalvorHaugan deleted the textarea-updates branch December 19, 2024 13:23
@github-actions github-actions bot mentioned this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants