Skip to content

Conversation

jouni
Copy link
Member

@jouni jouni commented Oct 10, 2025

Try to make it clear that these properties affect the text color, not the overall color (i.e., background) of a component or a part.

I also renamed the --vaadin-radio-button-dot-size to --vaadin-radio-button-marker-size to align with the marker-color property.

I discovered one left over --vaadin-subtle property reference as well, which seems like a batch-rename mistake from an earlier rename-refactoring.

@sissbruecker sissbruecker self-requested a review October 10, 2025 08:43
Copy link

vaadin-checkbox:is([checked], [indeterminate]):not([readonly], [disabled])::part(checkbox) {
--vaadin-checkbox-background: var(--aura-accent-color);
--vaadin-checkbox-color: var(--aura-accent-contrast);
--vaadin-checkbox-marker-color: var(--aura-accent-contrast);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lumo has --vaadin-checkbox-checkmark-color and --vaadin-checkbox-checkmark-size. Radio button also has --vaadin props with a different naming convention. Just checking that we are aware of the inconsistency. I guess those will be deprecated / removed in favor of the new ones at some point?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good point, thanks. I forgot to check that.

A bit unfortunate, as I’d like to add the "marker" to clarify the intent, but I don't like the breaking change. @rolfsmeds, shall we just keep the old name?

Copy link
Contributor

Choose a reason for hiding this comment

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

For reference, radio group has:

  • --vaadin-radio-button-dot-color
  • --vaadin-radio-button-marker-size

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, that one too. Maybe we should just revert those then, keep the old names.

Copy link
Contributor

Choose a reason for hiding this comment

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

There seem to be more inconsistencies. Lumo uses a --vaadin-checkbox-label-color, but there is no such thing in base styles or Aura.

Copy link
Member Author

@jouni jouni Oct 10, 2025

Choose a reason for hiding this comment

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

There is, it’s just abstracted together with radio-button as color: var(--vaadin-${unsafeCSS(propName)}-label-color, ...).

@jouni jouni requested a review from web-padawan October 13, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants