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: update selected item styles to not limit height #3548

Merged
merged 2 commits into from
Mar 14, 2022

Conversation

web-padawan
Copy link
Member

Description

Added styles to remove fixed height inherited from vaadin-button to display custom item properly.
Note, this only works in Lumo: multi-line items are not working using the Material theme for V14.

Fixes #3546

Type of change

  • Bugfix

@web-padawan web-padawan marked this pull request as draft March 11, 2022 12:06
@web-padawan web-padawan marked this pull request as ready for review March 11, 2022 12:43
@web-padawan web-padawan changed the title fix: reset height to make custom items fit fix: update selected item styles to not limit height Mar 11, 2022
@sonarcloud
Copy link

sonarcloud bot commented Mar 11, 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

@@ -23,7 +24,7 @@ registerStyles(

/* placeholder styles */
::slotted(:not([slot]):not([selected])) {
line-height: normal;
line-height: 1;
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason, using normal on Windows causes the input container height to increase by 1 pixel.
This change caused the placeholder screenshot to be updated for Lumo.

Copy link
Member

Choose a reason for hiding this comment

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

Are font descenders still visible with this change, for example, j and g?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can confirm that this works, single-line and multiline.

@@ -54,8 +55,8 @@ registerStyles(
'vaadin-select-value-button',
css`
:host {
min-height: var(--lumo-size-m);
Copy link
Member Author

Choose a reason for hiding this comment

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

This rule seems to be overridden by the vaadin-input-container styles.

Comment on lines +72 to +74
min-height: var(--lumo-button-size);
padding-top: var(--_lumo-selected-item-padding);
padding-bottom: var(--_lumo-selected-item-padding);
Copy link
Member Author

Choose a reason for hiding this comment

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

Aligned with the styles used by Vaadin 21:

:host([theme~='small']) [selected] {
padding: 0;
min-height: var(--lumo-size-s);
}

@sissbruecker sissbruecker merged commit a09a25c into master Mar 14, 2022
@sissbruecker sissbruecker deleted the fix/select-custom-item branch March 14, 2022 16:24
vaadin-bot pushed a commit that referenced this pull request Mar 14, 2022
* fix: reset height to make custom items fit

* fix: set correct slotted item height
sissbruecker pushed a commit that referenced this pull request Mar 14, 2022
* fix: reset height to make custom items fit

* fix: set correct slotted item height

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
@vaadin-bot
Copy link
Collaborator

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

Select component: padding removed with custom item renderer from v14 to v23
4 participants