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

[themable-mixin] Theme module for both vaadin-grid and vaadin-grid-pro doesn't apply to both #1579

Closed
pekam opened this issue Oct 16, 2019 · 4 comments

Comments

@pekam
Copy link
Contributor

pekam commented Oct 16, 2019

<dom-module id="foo" theme-for="vaadin-grid vaadin-grid-pro">
  <template>
    <style>
      [part~="cell"] {
        background: red;
      }
    </style>
  </template>
</dom-module>

Expected: <vaadin-grid> and <vaadin-grid-pro> both have red cells
Actual outcome: only <vaadin-grid> elements have red cells

Both elements have <style include="foo"></style> inside their shadow roots.

Discovered in: https://vaadin.com/forum/thread/17885978/theme-issue-for-gridpro-and-grid

@pekam
Copy link
Contributor Author

pekam commented Oct 16, 2019

The problem is in the order of including the styles. In normal case, the user-defined modules are loaded after the default styles. But because of extending grid, grid-pro includes styles in this order:

  1. include lumo styles for grid
  2. include foo for grid
  3. include lumo styles for grid-pro
  4. try to include foo for grid-pro, notice that it already exists and skip it (logic in _includeStyle function)

So the lumo styles for grid-pro are last actually included, and they will override foo styles.

@pekam
Copy link
Contributor Author

pekam commented Oct 16, 2019

Some workarounds:

  • use a stronger CSS-selector, e.g. [part~="row"] [part~="cell"]
  • use !important after the CSS-properties
  • define separate modules for each tag name

@pekam
Copy link
Contributor Author

pekam commented Oct 16, 2019

Suggested fix:

static _includeStyle(moduleName, template) {
  if (!template) {
    return;
  }
  let styleEl = template.content.querySelector(`style[include="${moduleName}"]`);
  if (!styleEl) {
    styleEl = document.createElement('style');
    styleEl.setAttribute('include', moduleName);
  }
  template.content.appendChild(styleEl);
}

So even when the style-module is already included, call appendChild to push the style-tag as last one.

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-themable-mixin May 19, 2021
@web-padawan web-padawan changed the title Theme module for both vaadin-grid and vaadin-grid-pro doesn't apply to both [themable-mixin] Theme module for both vaadin-grid and vaadin-grid-pro doesn't apply to both May 22, 2021
@tomivirkki
Copy link
Member

Works as expected with 24.3. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants