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: add Lumo custom properties for input field label #6636

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

web-padawan
Copy link
Member

@web-padawan web-padawan commented Oct 12, 2023

Description

Part of #2954

Type of change

  • Feature

CSS properties

The following custom CSS properties from the spreadsheet are added in this PR:

  • --vaadin-input-field-label-color
  • --vaadin-input-field-focused-label-color
  • --vaadin-input-field-hovered-label-color
  • --vaadin-input-field-label-font-size
  • --vaadin-input-field-label-font-weight

@@ -47,20 +43,12 @@ const inputField = css`
}

/* Hover */
:host(:hover:not([readonly]):not([focused])) [part='label'] {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved these styles to the file where [part='label'] are actually defined.

--_label-color-focused: var(--vaadin-input-field-focused-label-color, var(--lumo-primary-text-color));
--_label-color-hovered: var(--vaadin-input-field-hovered-label-color, var(--lumo-body-text-color));
--_label-font-size: var(--vaadin-input-field-label-font-size, var(--lumo-font-size-s));
--_label-font-weight: var(--vaadin-input-field-label-font-weight, 500);
Copy link
Contributor

Choose a reason for hiding this comment

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

Off topic: You also wanna align the --lumo properties I've added some time ago? (21d2556)

Copy link
Member Author

@web-padawan web-padawan Oct 13, 2023

Choose a reason for hiding this comment

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

Good question. We don't have these on the preliminary list at the moment, although some other --lumo- prefixed properties e.g. --lumo-button-size and --lumo-text-field-size are planned to be aligned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, I had forgotten about --lumo-required-field-indicator and --lumo-required-field-indicator-color.

I think that now that we're drawing a line between the abstract Lumo properties prefixed with --lumo- and the component/feature-specific properties prefixed with --vaadin-, it would make sense to refactor those to be --vaadin-prefixed (while keeping the current props around for backward compact, of course)

@web-padawan web-padawan marked this pull request as ready for review October 16, 2023 09:02
@@ -10,17 +10,26 @@ import '../typography.js';
import { css, registerStyles } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';

const requiredField = css`
:host {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this is another place where we don't really need these private intermediate properties that we decided to remove from the helper and error stylesheets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@sonarcloud
Copy link

sonarcloud bot commented Oct 18, 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 fe74383 into main Oct 19, 2023
9 checks passed
@web-padawan web-padawan deleted the feat/input-field-label-props branch October 19, 2023 11:51
@vaadin-bot
Copy link
Collaborator

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

4 participants