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

Theme variants for nested buttons #104

Closed
pekam opened this issue Apr 15, 2019 · 0 comments · Fixed by #105
Closed

Theme variants for nested buttons #104

pekam opened this issue Apr 15, 2019 · 0 comments · Fixed by #105
Assignees
Projects

Comments

@pekam
Copy link
Contributor

pekam commented Apr 15, 2019

Currently the theme variants work nicely with buttons inside a notification, so that e.g. placing a primary-button inside a primary-notification styles the button to have contrasted background: https://cdn.vaadin.com/vaadin-notification/1.3.0/demo/#notification-theme-variants-demos

However, because of the slotted selector, this works only if the button is a direct child of the <vaadin-notification-card>. Having any kind of wrapper element in-between breaks this styling. This is an issue for the Flow component, which uses <flow-component-renderer> + a <div> wrapper for the content elements.

A possible fix suggested by @tomivirkki is to add CSS properties for <vaadin-button>'s background and color:

:host {
 background-color: var(--lumo-button-background-color, var(--lumo-contrast-5pct));
 color: var(--lumo-button-color, var(--lumo-primary-text-color));
}

Then the <vaadin-notification-card> can override the properties:

:host([theme~="primary"]) {
  --lumo-button-background-color: var(--lumo-primary-contrast-color);
  --lumo-button-color: var(--lumo-primary-text-color);
}

UPDATE: The above solution doesn't actually work, because those variables should be changed only when the button inside the notification has primary theme. So we need a lot more properties, one for background and another for color per each theme attribute. Something like this:

<vaadin-button>

:host([theme~="primary"]) {
  background-color: var(--lumo-button-primary-background-color, var(--lumo-primary-color));
  color: var(--lumo-button-primary-color, var(--lumo-primary-contrast-color));

<vaadin-notification-card>

:host([theme~="primary"]) {
  --lumo-button-primary-background-color: var(--lumo-primary-contrast-color);
  --lumo-button-primary-color: var(--lumo-primary-text-color);
}

After implementing this in <vaadin-button>, verify that it works with notification and also in IE11, before releasing a new minor of button.

@web-padawan web-padawan added this to 🏃  Sprint in vaadin-core Apr 16, 2019
@pekam pekam self-assigned this Apr 18, 2019
pekam added a commit to vaadin/vaadin-button that referenced this issue Apr 18, 2019
to enable theming inside a notification,
connects to vaadin/vaadin-notification#104
pekam added a commit to vaadin/vaadin-button that referenced this issue Apr 18, 2019
for default and primary-themes, to enable theming inside a notification,
connects to vaadin/vaadin-notification#104
pekam added a commit that referenced this issue Apr 18, 2019
This enables vaadin-button styles to be applied for button's that are
not direct children of the vaadin-notification-card.

Fixes #104
pekam added a commit to vaadin/vaadin-button that referenced this issue Apr 18, 2019
for default and primary-themes, to enable theming inside a notification,
connects to vaadin/vaadin-notification#104
pekam added a commit that referenced this issue Apr 18, 2019
This enables vaadin-button styles to be applied for buttons that are
not direct children of the vaadin-notification-card.

Fixes #104
pekam added a commit that referenced this issue Apr 18, 2019
This enables vaadin-button styles to be applied for buttons that are
not direct children of the vaadin-notification-card.

Fixes #104
pekam added a commit to vaadin/vaadin-button that referenced this issue Apr 23, 2019
for default and primary-themes, to enable theming inside a notification,
connects to vaadin/vaadin-notification#104
vaadin-core automation moved this from 🏃  Sprint to ✅ Done Apr 26, 2019
pekam added a commit that referenced this issue Apr 26, 2019
This enables vaadin-button styles to be applied for buttons that are
not direct children of the vaadin-notification-card.

Fixes #104

Add workaround for IE11 CSS properties
@pekam pekam removed the in review label Apr 26, 2019
web-padawan pushed a commit to vaadin/web-components that referenced this issue Mar 1, 2021
for default and primary-themes, to enable theming inside a notification,
connects to vaadin/vaadin-notification#104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
vaadin-core
  
✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants