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

fix: position the material text-area bottom bar #3359

Merged
merged 5 commits into from
Feb 1, 2022

Conversation

tomivirkki
Copy link
Member

@tomivirkki tomivirkki commented Jan 31, 2022

Fixes #1333
Closes #3241

The fix is inspired by #1333 (comment)

There's now a private CSS variable named --_text-area-vertical-scroll-position which is kept in sync with the scrollTop of the input field. It's used for adjusting the position of the pseudo-elements used in MD for the bottom bar.

@tomivirkki tomivirkki marked this pull request as draft January 31, 2022 11:06
@tomivirkki tomivirkki force-pushed the fix-material-text-area-bar-position branch from 054b0b2 to 1cd8bca Compare January 31, 2022 12:33
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM. When removing max-height set on the element, scrollbar does not go away automatically. This is an edge case so we can probably address it later (it will probably require using ResizeObserver).

scroll.mp4

.

@sonarcloud
Copy link

sonarcloud bot commented Feb 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tomivirkki
Copy link
Member Author

LGTM. When removing max-height set on the element, scrollbar does not go away automatically. This is an edge case so we can probably address it later (it will probably require using ResizeObserver).

Good catch. There was a unit test for resizing but this bug turned out to be theme-specific. Added ResizeMixin as suggested and now it works as expected.

@tomivirkki tomivirkki merged commit a4ba585 into master Feb 1, 2022
@tomivirkki tomivirkki deleted the fix-material-text-area-bar-position branch February 1, 2022 14:44
@vaadin-bot
Copy link
Collaborator

Hi @tomivirkki , this commit cannot be picked to 22.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick a4ba585
error: could not apply a4ba585... fix: position the material text-area bottom bar (#3359)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 23.0.0.beta2 and is also targeting the upcoming stable 23.0.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.

[text-field] Broken vaadin-text-area scroll in Material theme
3 participants