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: set relative positioning explicitly on scroll containers #5714

Merged

Conversation

bwajtr
Copy link
Contributor

@bwajtr bwajtr commented Mar 24, 2023

Description

This PR fixes the showing of the horizontal scrollbar in ComboBoxes (and other components) in Firefox (version greater or equal 105) when the component is put into a more complex web component structure. For more details on how to reliably reproduce the issue and more details please see this comment.

Just a short recap of the root cause:

The root cause of the issue is the fact that during the ComboBox creation, when we are creating the Virtualizer for it, at a certain point in code, we call getComputedStyle() on several elements of combobox, expecting to see some values and acting on it. However, Firefox in this situation returns empty styles object where every style property is an empty string instead of some default value. This is unexpected by the code and causes the issue. Other browsers do not behave like that.

We were discussing the possiblity to fix the issue directly in the Javascript code in here:

if (getComputedStyle(this.scrollContainer).position === 'static') {
this.scrollContainer.style.position = 'relative';
}

Where we could check if getComputedStyle() returned an empty value. However, after some discussions, we decided to fix the issue by explicitly setting the position: relative in CSS for each of the affected components, as it is more understandable fix.

Fixes #5590

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@web-padawan
Copy link
Member

However, Firefox in this situation returns empty styles object where every style property is an empty string instead of some default value.

Thanks for a detailed clarification. I recall seeing something like this earlier during debugging.

@web-padawan web-padawan force-pushed the 5590-horizontal-scrollbar-in-virtualizer-enabled-components branch from 9efcb53 to e2c898e Compare March 27, 2023 14:09
@sonarcloud
Copy link

sonarcloud bot commented Mar 27, 2023

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

@web-padawan web-padawan merged commit 57a8caf into main Mar 27, 2023
@web-padawan web-padawan deleted the 5590-horizontal-scrollbar-in-virtualizer-enabled-components branch March 27, 2023 14:50
@vaadin-bot
Copy link
Collaborator

Hi @bwajtr , this commit cannot be picked to 23.3 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 57a8caf
error: could not apply 57a8caf... fix: explicitly set relative positioning on scroll containers (#5714)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.1.0.alpha2 and is also targeting the upcoming stable 24.1.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.

Horizontal scrollbar in vaadin-combo-box in Firefox when used in lit component
5 participants