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

feat: configure maximum number of rows in text area #8143

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

sissbruecker
Copy link
Contributor

Allows to configure the maximum number of rows before the component starts scrolling.

Part of #1399

@@ -209,6 +219,36 @@ export const TextAreaMixin = (superClass) =>
}
}

/** @private */
__updateMaxHeight(maxRows) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the best I could think of, though it has some flaws:

  • Borders are ignored, though it seems unlikely that they would be used in inner parts of the component. We could include them if it becomes necessary.
  • If line-height computes to a non-numeric value such as normal, then effectively no max height will be set. That seems to only be the case if normal is set explicitly.

I was also considering implementing some measurement similar to what is done in _updateHeight. That would involve temporarily setting rows to the max. number of rows and then measuring the resulting height of the input container, plus a lot of stuff around that to prepare for measurement and restoring some initial values after. That felt more complex and brittle overall.

But maybe I'm missing something crucial with this approach, feel free to suggest alternatives if you can think of some.

Copy link

sonarcloud bot commented Nov 18, 2024

@sissbruecker sissbruecker merged commit f3f7ed5 into main Nov 18, 2024
9 checks passed
@sissbruecker sissbruecker deleted the feat/text-area-max-rows branch November 18, 2024 11:45
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.6.0.alpha4 and is also targeting the upcoming stable 24.6.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants